revision of fix_fields() calls (BUG2838)

mysql-test/r/subselect.result

@@ -1614,3 +1614,9 @@ select 2 in (select * from t1);
 1
 SET SQL_SELECT_LIMIT=default;
 drop table t1;
+create table t1(val varchar(10));
+insert into t1 values ('aaa'), ('bbb'),('eee'),('mmm'),('ppp');
+select count(*) from t1 as w1 where w1.val in (select w2.val from t1 as w2 where w2.val like 'm%') and w1.val in (select w3.val from t1 as w3 where w3.val like 'e%');
+count(*)
+0
+drop table if exists t1;

mysql-test/t/subselect.test

@@ -1060,3 +1060,11 @@ select sum(a) from (select * from t1) as a;
 select 2 in (select * from t1);
 SET SQL_SELECT_LIMIT=default;
 drop table t1;
+
+#
+# Item_cond fix field
+#
+create table t1(val varchar(10));
+insert into t1 values ('aaa'), ('bbb'),('eee'),('mmm'),('ppp');
+select count(*) from t1 as w1 where w1.val in (select w2.val from t1 as w2 where w2.val like 'm%') and w1.val in (select w3.val from t1 as w3 where w3.val like 'e%');
+drop table if exists t1;

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;

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();

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

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;
 }

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);

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;

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);

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;

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 &&

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);
   }

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)
         {