A fix and test case for Bug#5987 "subselect in bool function

crashes server (prepared statements)": the bug was that all boolean
items always recovered its original arguments at statement cleanup 
stage.
This collided with Item_subselect::select_transformer, which tries to 
permanently change the item tree to use a transformed subselect instead of
original one.
So we had this call sequence for prepare:
mysql_stmt_prepare -> JOIN::prepare -> 
Item_subselect::fix_fields -> the item tree gets transformed ->
Item_bool_rowready_func2::cleanup, item tree is recovered to original
state, while it shouldn't have been;
mysql_stmt_execute -> attempts to execute a broken tree -> crash.
Now instead of bluntly recovering all arguments of bool functions in 
Item_bool_rowready_func2::cleanup, we recover only those
which were changed, and do it in one place.
There still would exist a possibility for a collision with subselect
tranformation, if permanent and temporary changes were performed at the 
same stage.
But fortunately subselect transformation is always done first, so it 
doesn't conflict with the optimization done by propogate_cond_constants.
Now we have: 
mysql_stmt_prepare -> JOIN::prepare -> subselect transformation 
permanently changes the tree -> cleanup doesn't recover anything, 
because nothing was registered for recovery.
mysql_stmt_execute -> JOIN::prepare (the tree is already transformed, 
so it doesn't change), JOIN::optimize -> 
propogate_cond_constants -> temporary changes the item tree 
with constants -> JOIN::execute -> cleanup -> 
the changes done by propogate_cond_constants are recovered, as
they were registered for recovery.

mysql-test/r/ps.result

@@ -308,3 +308,13 @@ execute stmt using @a, @a;
 a
 drop table t1;
 deallocate prepare stmt;
+create table t1 (a int);
+prepare stmt from "select * from t1 where 1 > (1 in (SELECT * FROM t1))";
+execute stmt;
+a
+execute stmt;
+a
+execute stmt;
+a
+drop table t1;
+deallocate prepare stmt;

mysql-test/t/ps.test

@@ -329,3 +329,17 @@ execute stmt using @a, @a;
 drop table t1;
 deallocate prepare stmt;
 
+#
+# Bug #5987 subselect in bool function crashes server (prepared statements):
+# don't overwrite transformed subselects with old arguments of a bool
+# function.
+#
+create table t1 (a int);
+prepare stmt from "select * from t1 where 1 > (1 in (SELECT * FROM t1))";
+execute stmt;
+execute stmt;
+execute stmt;
+drop table t1;
+deallocate prepare stmt;
+
+

sql/item.cc

@@ -2209,10 +2209,12 @@ Item_result item_cmp_type(Item_result a,Item_result b)
 }
 
 
-Item *resolve_const_item(Item *item,Item *comp_item)
+void resolve_const_item(THD *thd, Item **ref, Item *comp_item)
 {
+  Item *item= *ref;
+  Item *new_item;
   if (item->basic_const_item())
-    return item;				// Can't be better
+    return;                                     // Can't be better
   Item_result res_type=item_cmp_type(comp_item->result_type(),
 				     item->result_type());
   char *name=item->name;			// Alloced by sql_alloc
@@ -2223,26 +2225,34 @@ Item *resolve_const_item(Item *item,Item *comp_item)
     String tmp(buff,sizeof(buff),&my_charset_bin),*result;
     result=item->val_str(&tmp);
     if (item->null_value)
-      return new Item_null(name);
-    uint length=result->length();
-    char *tmp_str=sql_strmake(result->ptr(),length);
-    return new Item_string(name,tmp_str,length,result->charset());
+      new_item= new Item_null(name);
+    else
+    {
+      uint length= result->length();
+      char *tmp_str= sql_strmake(result->ptr(), length);
+      new_item= new Item_string(name, tmp_str, length, result->charset());
+    }
   }
-  if (res_type == INT_RESULT)
+  else if (res_type == INT_RESULT)
   {
     longlong result=item->val_int();
     uint length=item->max_length;
     bool null_value=item->null_value;
-    return (null_value ? (Item*) new Item_null(name) :
-	    (Item*) new Item_int(name,result,length));
+    new_item= (null_value ? (Item*) new Item_null(name) :
+               (Item*) new Item_int(name, result, length));
   }
   else
   {						// It must REAL_RESULT
     double result=item->val();
     uint length=item->max_length,decimals=item->decimals;
     bool null_value=item->null_value;
-    return (null_value ? (Item*) new Item_null(name) :
-	    (Item*) new Item_real(name,result,decimals,length));
+    new_item= (null_value ? (Item*) new Item_null(name) : (Item*)
+               new Item_real(name, result, decimals, length));
+  }
+  if (new_item)
+  {
+    thd->register_item_tree_change(ref, item, &thd->mem_root);
+    *ref= new_item;
   }
 }
 

sql/item.h

@@ -1223,5 +1223,5 @@ public:
 
 extern Item_buff *new_Item_buff(Item *item);
 extern Item_result item_cmp_type(Item_result a,Item_result b);
-extern Item *resolve_const_item(Item *item,Item *cmp_item);
+extern void resolve_const_item(THD *thd, Item **ref, Item *cmp_item);
 extern bool field_is_equal_to_item(Field *field,Item *item);

sql/item_cmpfunc.h

@@ -210,21 +210,11 @@ public:
 
 class Item_bool_rowready_func2 :public Item_bool_func2
 {
-  Item *orig_a, *orig_b; /* propagate_const can change parameters */
 public:
-  Item_bool_rowready_func2(Item *a,Item *b) :Item_bool_func2(a,b),
-    orig_a(a), orig_b(b)
+  Item_bool_rowready_func2(Item *a, Item *b) :Item_bool_func2(a, b)
   {
     allowed_arg_cols= a->cols();
   }
-  void cleanup()
-  {
-    DBUG_ENTER("Item_bool_rowready_func2::cleanup");
-    Item_bool_func2::cleanup();
-    tmp_arg[0]= orig_a;
-    tmp_arg[1]= orig_b;
-    DBUG_VOID_RETURN;
-  }
   Item *neg_transformer(THD *thd);
   virtual Item *negated_item();
 };

sql/sql_class.cc

@@ -730,7 +730,7 @@ void THD::nocheck_register_item_tree_change(Item **place, Item *old_value,
   change= new (change_mem) Item_change_record;
   change->place= place;
   change->old_value= old_value;
-  change_list.push_back(change);
+  change_list.append(change);
 }
 
 

sql/sql_select.cc

@@ -4171,8 +4171,9 @@ template class List_iterator<Item_func_match>;
 */
 
 static void
-change_cond_ref_to_const(I_List<COND_CMP> *save_list,Item *and_father,
-			 Item *cond, Item *field, Item *value)
+change_cond_ref_to_const(THD *thd, I_List<COND_CMP> *save_list,
+                         Item *and_father, Item *cond,
+                         Item *field, Item *value)
 {
   if (cond->type() == Item::COND_ITEM)
   {
@@ -4181,7 +4182,7 @@ change_cond_ref_to_const(I_List<COND_CMP> *save_list,Item *and_father,
     List_iterator<Item> li(*((Item_cond*) cond)->argument_list());
     Item *item;
     while ((item=li++))
-      change_cond_ref_to_const(save_list,and_level ? cond : item, item,
+      change_cond_ref_to_const(thd, save_list,and_level ? cond : item, item,
 			       field, value);
     return;
   }
@@ -4189,8 +4190,9 @@ change_cond_ref_to_const(I_List<COND_CMP> *save_list,Item *and_father,
     return;					// Not a boolean function
 
   Item_bool_func2 *func=  (Item_bool_func2*) cond;
-  Item *left_item=  func->arguments()[0];
-  Item *right_item= func->arguments()[1];
+  Item **args= func->arguments();
+  Item *left_item=  args[0];
+  Item *right_item= args[1];
   Item_func::Functype functype=  func->functype();
 
   if (right_item->eq(field,0) && left_item != value &&
@@ -4201,7 +4203,8 @@ change_cond_ref_to_const(I_List<COND_CMP> *save_list,Item *and_father,
     Item *tmp=value->new_item();
     if (tmp)
     {
-      func->arguments()[1] = tmp;
+      thd->register_item_tree_change(args + 1, args[1], &thd->mem_root);
+      args[1]= tmp;
       func->update_used_tables();
       if ((functype == Item_func::EQ_FUNC || functype == Item_func::EQUAL_FUNC)
 	  && and_father != cond && !left_item->const_item())
@@ -4222,13 +4225,15 @@ change_cond_ref_to_const(I_List<COND_CMP> *save_list,Item *and_father,
     Item *tmp=value->new_item();
     if (tmp)
     {
-      func->arguments()[0] = value = tmp;
+      thd->register_item_tree_change(args, args[0], &thd->mem_root);
+      args[0]= value= tmp;
       func->update_used_tables();
       if ((functype == Item_func::EQ_FUNC || functype == Item_func::EQUAL_FUNC)
 	  && and_father != cond && !right_item->const_item())
       {
-	func->arguments()[0] = func->arguments()[1]; // For easy check
-	func->arguments()[1] = value;
+        args[0]= args[1];                       // For easy check
+        thd->register_item_tree_change(args + 1, args[1], &thd->mem_root);
+        args[1]= value;
 	cond->marker=1;
 	COND_CMP *tmp2;
 	if ((tmp2=new COND_CMP(and_father,func)))
@@ -4274,8 +4279,8 @@ static Item *remove_additional_cond(Item* conds)
 }
 
 static void
-propagate_cond_constants(I_List<COND_CMP> *save_list,COND *and_father,
-			 COND *cond)
+propagate_cond_constants(THD *thd, I_List<COND_CMP> *save_list,
+                         COND *and_father, COND *cond)
 {
   if (cond->type() == Item::COND_ITEM)
   {
@@ -4286,18 +4291,19 @@ propagate_cond_constants(I_List<COND_CMP> *save_list,COND *and_father,
     I_List<COND_CMP> save;
     while ((item=li++))
     {
-      propagate_cond_constants(&save,and_level ? cond : item, item);
+      propagate_cond_constants(thd, &save,and_level ? cond : item, item);
     }
     if (and_level)
     {						// Handle other found items
       I_List_iterator<COND_CMP> cond_itr(save);
       COND_CMP *cond_cmp;
       while ((cond_cmp=cond_itr++))
-	if (!cond_cmp->cmp_func->arguments()[0]->const_item())
-	  change_cond_ref_to_const(&save,cond_cmp->and_level,
-				   cond_cmp->and_level,
-				   cond_cmp->cmp_func->arguments()[0],
-				   cond_cmp->cmp_func->arguments()[1]);
+      {
+        Item **args= cond_cmp->cmp_func->arguments();
+        if (!args[0]->const_item())
+          change_cond_ref_to_const(thd, &save,cond_cmp->and_level,
+                                   cond_cmp->and_level, args[0], args[1]);
+      }
     }
   }
   else if (and_father != cond && !cond->marker)		// In a AND group
@@ -4307,29 +4313,25 @@ propagate_cond_constants(I_List<COND_CMP> *save_list,COND *and_father,
 	 ((Item_func*) cond)->functype() == Item_func::EQUAL_FUNC))
     {
       Item_func_eq *func=(Item_func_eq*) cond;
-      bool left_const= func->arguments()[0]->const_item();
-      bool right_const=func->arguments()[1]->const_item();
+      Item **args= func->arguments();
+      bool left_const= args[0]->const_item();
+      bool right_const= args[1]->const_item();
       if (!(left_const && right_const) &&
-	  (func->arguments()[0]->result_type() ==
-	   (func->arguments()[1]->result_type())))
+          args[0]->result_type() == args[1]->result_type())
       {
 	if (right_const)
 	{
-	  func->arguments()[1]=resolve_const_item(func->arguments()[1],
-						  func->arguments()[0]);
+          resolve_const_item(thd, &args[1], args[0]);
 	  func->update_used_tables();
-	  change_cond_ref_to_const(save_list,and_father,and_father,
-				   func->arguments()[0],
-				   func->arguments()[1]);
+          change_cond_ref_to_const(thd, save_list, and_father, and_father,
+                                   args[0], args[1]);
 	}
 	else if (left_const)
 	{
-	  func->arguments()[0]=resolve_const_item(func->arguments()[0],
-						  func->arguments()[1]);
+          resolve_const_item(thd, &args[0], args[1]);
 	  func->update_used_tables();
-	  change_cond_ref_to_const(save_list,and_father,and_father,
-				   func->arguments()[1],
-				   func->arguments()[0]);
+          change_cond_ref_to_const(thd, save_list, and_father, and_father,
+                                   args[1], args[0]);
 	}
       }
     }
@@ -4346,7 +4348,7 @@ optimize_cond(THD *thd, COND *conds, Item::cond_result *cond_value)
   {
     DBUG_EXECUTE("where", print_where(conds, "original"););
     /* change field = field to field = const for each found field = const */
-    propagate_cond_constants((I_List<COND_CMP> *) 0, conds, conds);
+    propagate_cond_constants(thd, (I_List<COND_CMP> *) 0, conds, conds);
     /*
       Remove all instances of item == item
       Remove all and-levels where CONST item != CONST item