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.

5 files changed, 58 insertions, 56 deletions

diff --git a/sql/item.cc b/sql/item.cc
index 564bc72927c..3165f48aa8f 100644
--- a/sql/item.cc
+++ b/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;
   }
 }
 
diff --git a/sql/item.h b/sql/item.h
index 589edb88565..e1dd7301217 100644
--- a/sql/item.h
+++ b/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);
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index 3e98684b6ca..dcc32441e88 100644
--- a/sql/item_cmpfunc.h
+++ b/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();
 };
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 5273d379841..4ce655c78f8 100644
--- a/sql/sql_class.cc
+++ b/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);
 }
 
 
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 0834f0659da..f2d814ed057 100644
--- a/sql/sql_select.cc
+++ b/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