revision of fix_fields() calls (BUG2838)

mysql-test/r/subselect.result:
  test suite for BUG#2838
mysql-test/t/subselect.test:
  test suite for BUG#2838
sql/item.cc:
  revision of fix_fields() calls
  removed unnecessary variabl, and used correct last parameter of fix_fields()
sql/item_cmpfunc.cc:
  revision of fix_fields() calls (BUG#2838)
sql/item_func.cc:
  revision of fix_fields() calls
sql/item_row.cc:
  changed for efficience (and to be similar for Item_func::fix_fields)
sql/item_subselect.cc:
  fixed last arguments for fix_fields call
sql/item_sum.cc:
  layout fixed
  revision of fix_fields() calls
sql/log_event.cc:
  revision of fix_fields() calls
sql/set_var.cc:
  revision of fix_fields() calls
sql/sql_base.cc:
  revision of fix_fields() calls
sql/sql_class.cc:
  revision of fix_fields() calls
sql/sql_handler.cc:
  revision of fix_fields() calls

11 files changed, 61 insertions, 36 deletions

diff --git a/sql/item.cc b/sql/item.cc
index eb2550fdbcc..9cf1ccb03ed 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -928,6 +928,10 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 			       (char *)field_name);
 	if (!rf)
 	  return 1;
+	/*
+	  rf is Item_ref => never substitute other items (in this case)
+	  during fix_fields() => we can use rf after fix_fields()
+	*/
 	if (rf->fix_fields(thd, tables, ref) || rf->check_cols(1))
 	  return 1;
 
@@ -946,6 +950,10 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 				 (char *)field_name);
 	  if (!rf)
 	    return 1;
+	  /*
+	    rf is Item_ref => never substitute other items (in this case)
+	    during fix_fields() => we can use rf after fix_fields()
+	  */
 	  return rf->fix_fields(thd, tables, ref) ||  rf->check_cols(1);
 	}
       }
@@ -1654,16 +1662,15 @@ bool Item_default_value::eq(const Item *item, bool binary_cmp) const
 }
 
 
-bool Item_default_value::fix_fields(THD *thd, struct st_table_list *table_list, Item **items)
+bool Item_default_value::fix_fields(THD *thd,
+				    struct st_table_list *table_list,
+				    Item **items)
 {
   if (!arg)
-    return false;
-  bool res= arg->fix_fields(thd, table_list, items);
-  if (res)
-    return res;
-  /* arg->type() can be only REF_ITEM or FIELD_ITEM for it defined as
-   simple_ident in sql_yacc.yy
-  */
+    return 0;
+  if (arg->fix_fields(thd, table_list, &arg))
+    return 1;
+  
   if (arg->type() == REF_ITEM)
   {
     Item_ref *ref= (Item_ref *)arg;
@@ -1707,13 +1714,9 @@ bool Item_insert_value::fix_fields(THD *thd,
 				   struct st_table_list *table_list,
 				   Item **items)
 {
-  bool res= arg->fix_fields(thd, table_list, items);
-  if (res)
-    return res;
-  /*
-    arg->type() can be only REF_ITEM or FIELD_ITEM as arg is
-    a simple_ident in sql_yacc.yy
-  */
+  if (arg->fix_fields(thd, table_list, &arg))
+    return 1;
+
   if (arg->type() == REF_ITEM)
   {
     Item_ref *ref= (Item_ref *)arg;
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 1bba934cf8f..237cb6bf0de 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -1755,8 +1755,11 @@ Item_cond::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
     }
     if (abort_on_null)
       item->top_level_item();
+
+    // item can be substituted in fix_fields
     if ((!item->fixed &&
-	 item->fix_fields(thd, tables, li.ref())) || item->check_cols(1))
+	 item->fix_fields(thd, tables, li.ref())) ||
+	(item= *li.ref())->check_cols(1))
       return 1; /* purecov: inspected */
     used_tables_cache|=     item->used_tables();
     tmp_table_map=	    item->not_null_tables();
diff --git a/sql/item_func.cc b/sql/item_func.cc
index 0327204dbfd..817bd2b866b 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -1469,8 +1469,11 @@ udf_handler::fix_fields(THD *thd, TABLE_LIST *tables, Item_result_field *func,
 	 arg != arg_end ;
 	 arg++,i++)
     {
+      if ((*arg)->fix_fields(thd, tables, arg))
+	return 1;
+      // we can't assign 'item' before, because fix_fields() can change arg
       Item *item= *arg;
-      if (item->fix_fields(thd, tables, arg) || item->check_cols(1))
+      if (item->check_cols(1))
 	return 1;
       /*
 	TODO: We should think about this. It is not always
diff --git a/sql/item_row.cc b/sql/item_row.cc
index 7f847bd1d4e..2b0cd38e3c1 100644
--- a/sql/item_row.cc
+++ b/sql/item_row.cc
@@ -62,19 +62,21 @@ bool Item_row::fix_fields(THD *thd, TABLE_LIST *tabl, Item **ref)
   {
     if ((*arg)->fix_fields(thd, tabl, arg))
       return 1;
-    used_tables_cache |= (*arg)->used_tables();
-    if (const_item_cache&= (*arg)->const_item() && !with_null)
+    // we can't assign 'item' before, because fix_fields() can change arg
+    Item *item= *arg;
+    used_tables_cache |= item->used_tables();
+    if (const_item_cache&= item->const_item() && !with_null)
     {
-      if ((*arg)->cols() > 1)
-	with_null|= (*arg)->null_inside();
+      if (item->cols() > 1)
+	with_null|= item->null_inside();
       else
       {
-	(*arg)->val_int();
-	with_null|= (*arg)->null_value;
+	item->val_int();
+	with_null|= item->null_value;
       }
     }
-    maybe_null|= (*arg)->maybe_null;
-    with_sum_func= with_sum_func || (*arg)->with_sum_func;
+    maybe_null|= item->maybe_null;
+    with_sum_func= with_sum_func || item->with_sum_func;
   }
   return 0;
 }
diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc
index 5b3cc326679..54676e0ade9 100644
--- a/sql/item_subselect.cc
+++ b/sql/item_subselect.cc
@@ -597,7 +597,8 @@ Item_in_subselect::single_value_transformer(JOIN *join,
       select_lex->item_list.empty();
       select_lex->item_list.push_back(item);
     
-      if (item->fix_fields(thd_tmp, join->tables_list, &item))
+      if (item->fix_fields(thd_tmp, join->tables_list,
+			   select_lex->item_list.head_ref()))
       {
 	DBUG_RETURN(RES_ERROR);
       }
@@ -819,7 +820,7 @@ Item_in_subselect::row_value_transformer(JOIN *join)
   else
   {
     join->conds= and_items(join->conds, item);
-    if (join->conds->fix_fields(thd_tmp, join->tables_list, &join->having))
+    if (join->conds->fix_fields(thd_tmp, join->tables_list, &join->conds))
       DBUG_RETURN(RES_ERROR);
   }
   DBUG_RETURN(RES_OK);
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index 10b50fa5b3b..762c2df6cf7 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -191,17 +191,21 @@ Item_sum_num::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 bool
 Item_sum_hybrid::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 {
-  Item *item=args[0];
+  Item *item= args[0];
   if (!thd->allow_sum_func)
   {
     my_error(ER_INVALID_GROUP_FUNC_USE,MYF(0));
     return 1;
   }
   thd->allow_sum_func=0;			// No included group funcs
+
+  // 'item' can be changed during fix_fields
   if (!item->fixed &&
-      item->fix_fields(thd, tables, args) || item->check_cols(1))
+      item->fix_fields(thd, tables, args) ||
+      (item= args[0])->check_cols(1))
     return 1;
-  hybrid_type=item->result_type();
+
+  hybrid_type= item->result_type();
   if (hybrid_type == INT_RESULT)
   {
     cmp_charset= &my_charset_bin;
diff --git a/sql/log_event.cc b/sql/log_event.cc
index a128b5809b2..0bcaff22f0f 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -2344,6 +2344,10 @@ int User_var_log_event::exec_event(struct st_relay_log_info* rli)
     }
   }
   Item_func_set_user_var e(user_var_name, it);
+  /*
+    Item_func_set_user_var can't substitute something else on its place =>
+    0 can be passed as last argument (reference on item)
+  */
   e.fix_fields(thd, 0, 0);
   e.update_hash(val, val_len, type, charset, DERIVATION_NONE);
   free_root(&thd->mem_root,0);
diff --git a/sql/set_var.cc b/sql/set_var.cc
index 7f5cf9503bf..c6dcaa8a6fb 100644
--- a/sql/set_var.cc
+++ b/sql/set_var.cc
@@ -2457,7 +2457,7 @@ int set_var_user::check(THD *thd)
 {
   /*
     Item_func_set_user_var can't substitute something else on its place =>
-    0 can be passed as last argument
+    0 can be passed as last argument (reference on item)
   */
   return (user_var_item->fix_fields(thd, 0, (Item**) 0) ||
 	  user_var_item->check()) ? -1 : 0;
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index b11c71b6ff7..ff55f88d5ec 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -2127,9 +2127,8 @@ int setup_fields(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
   while ((item= it++))
   {
     if (item->fix_fields(thd, tables, it.ref()) ||
-	item->check_cols(1))
+	(item= *(it.ref()))->check_cols(1))
       DBUG_RETURN(-1); /* purecov: inspected */
-    item= *(it.ref()); //Item can be changed in fix fields
     if (ref)
       *(ref++)= item;
     if (item->with_sum_func && item->type() != Item::SUM_FUNC_ITEM &&
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index e7f867ccd61..c5083bd2dcb 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -1176,8 +1176,12 @@ int select_dumpvar::prepare(List<Item> &list, SELECT_LEX_UNIT *u)
   {
     ls= gl++;
     Item_func_set_user_var *xx = new Item_func_set_user_var(*ls,item);
+    /*
+      Item_func_set_user_var can't substitute something else on its place =>
+      0 can be passed as last argument (reference on item)
+    */
     xx->fix_fields(thd,(TABLE_LIST*) thd->lex->select_lex.table_list.first,
-		   &item);
+		   0);
     xx->fix_length_and_dec();
     vars.push_back(xx);
   }
diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc
index d0f241b3291..881f75566cd 100644
--- a/sql/sql_handler.cc
+++ b/sql/sql_handler.cc
@@ -207,11 +207,13 @@ int mysql_ha_read(THD *thd, TABLE_LIST *tables,
 			MYF(0),keyinfo->key_parts);
 	goto err;
       }
-      List_iterator_fast<Item> it_ke(*key_expr);
+      List_iterator<Item> it_ke(*key_expr);
       Item *item;
       for (key_len=0 ; (item=it_ke++) ; key_part++)
       {
-	if (item->fix_fields(thd, tables, &item))
+	// 'item' can be changed by fix_fields() call
+	if (item->fix_fields(thd, tables, it_ke.ref()) ||
+	    (item= *it_ke.ref())->check_cols(1))
 	  goto err;
 	if (item->used_tables() & ~RAND_TABLE_BIT)
         {