bk commit into 5.0 tree (kroki:1.2243) BUG#21166

[kroki]

Below is the list of changes that have just been committed into a local
5.0 repository of tomash. When tomash does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2006-08-24 15:49:12+04:00, kroki@stripped +9 -0
  BUG#21166: Prepared statement causes signal 11 on second execution
  
  Changes in an item tree done by optimizer weren't properly
  registered and went unnoticed, which resulted in preliminary freeing
  of used memory.

  mysql-test/r/ps.result@stripped, 2006-08-24 15:49:06+04:00, kroki@stripped +15 -0
    Add result for bug#21166: Prepared statement causes signal 11
    on second execution.

  mysql-test/t/ps.test@stripped, 2006-08-24 15:49:06+04:00, kroki@stripped +30 -1
    Add test case for bug#21166: Prepared statement causes signal 11
    on second execution.

  sql/item.cc@stripped, 2006-08-24 15:49:06+04:00, kroki@stripped +70 -2
    Move Item::transform() and Item_default_value::transform() from
    item.h here and use THD::change_item_tree() instead of plain
    assignment.
    Change Item_field::set_no_const_sub() to be used with Item::walk()
    instead of Item::transform().

  sql/item.h@stripped, 2006-08-24 15:49:07+04:00, kroki@stripped +4 -18
    Move definition of Item::transform() and Item_default_value::transform()
    to item.cc.
    Change Item::set_no_const_sub() to be used with Item::walk()
    instead of Item::transform().

  sql/item_cmpfunc.cc@stripped, 2006-08-24 15:49:08+04:00, kroki@stripped +22 -4
    Use Item::walk() to execute Item::set_no_const_sub().
    Use THD::change_item_tree() instead of plain assignment.

  sql/item_func.cc@stripped, 2006-08-24 15:49:08+04:00, kroki@stripped +9 -0
    Add assert and comment to Item_func::traverse_cond().

  sql/item_row.cc@stripped, 2006-08-24 15:49:08+04:00, kroki@stripped +11 -1
    Use THD::change_item_tree() instead of plain assignment.

  sql/item_strfunc.cc@stripped, 2006-08-24 15:49:08+04:00, kroki@stripped +20 -0
    Move Item_func_make_set::transform() from item_strfunc.h here and use
    THD::change_item_tree() instead of plain assignment.

  sql/item_strfunc.h@stripped, 2006-08-24 15:49:08+04:00, kroki@stripped +1 -8
    Move definition of Item_func_make_set::transform() to item_strfunc.cc.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	kroki
# Host:	moonlight.intranet
# Root:	/home/tomash/src/mysql_ab/mysql-5.0-bug21166

--- 1.230/sql/item.cc	2006-08-24 15:49:20 +04:00
+++ 1.231/sql/item.cc	2006-08-24 15:49:20 +04:00
@@ -420,6 +420,50 @@ void Item::rename(char *new_name)
 }
 
 
+/*
+  transform() - traverse item tree possibly transforming it (replacing
+                items)
+
+  SYNOPSIS
+    transform()
+      transformer    functor that performs transformation of a subtree
+      arg            opaque argument passed to the functor
+
+  DESCRIPTION
+    This function is designed to ease transformation of Item trees.
+
+    Re-execution note: every such transformation is registered for
+    rollback by THD::change_item_tree() and is rolled back at the end
+    of execution by THD::rollback_item_tree_changes().
+
+    Therefore:
+
+      - this function can not be used at prepared statement prepare
+        (in particular, in fix_fields!), as only permanent
+        transformation of Item trees are allowed at prepare.
+
+      - the transformer function shall allocate new Items in execution
+        memory root (thd->mem_root) and not anywhere else: allocated
+        items will be gone in the end of execution.
+
+    If you don't need to transform an item tree, but only traverse
+    it, please use Item::walk() instead.
+
+
+  RETURN
+    Returns pointer to the new subtree root.  THD::change_item_tree()
+    should be called for it if transformation took place, i.e. if
+    pointer to newly allocated item is returned.
+*/
+
+Item* Item::transform(Item_transformer transformer, byte *arg)
+{
+  DBUG_ASSERT(!current_thd->is_stmt_prepare());
+
+  return (this->*transformer)(arg);
+}
+
+
 Item_ident::Item_ident(Name_resolution_context *context_arg,
                        const char *db_name_arg,const char *table_name_arg,
 		       const char *field_name_arg)
@@ -3788,11 +3832,11 @@ Item *Item_field::equal_fields_propagato
   See comments in Arg_comparator::set_compare_func() for details
 */
 
-Item *Item_field::set_no_const_sub(byte *arg)
+bool Item_field::set_no_const_sub(byte *arg)
 {
   if (field->charset() != &my_charset_bin)
     no_const_subst=1;
-  return this;
+  return FALSE;
 }
 
 
@@ -5291,6 +5335,30 @@ int Item_default_value::save_in_field(Fi
     return 0;
   }
   return Item_field::save_in_field(field_arg, no_conversions);
+}
+
+
+/* 
+   This method like the walk method traverses the item tree, but at
+   the same time it can replace some nodes in the tree
+*/ 
+Item *Item_default_value::transform(Item_transformer transformer, byte *args)
+{
+  DBUG_ASSERT(!current_thd->is_stmt_prepare());
+
+  Item *new_item= arg->transform(transformer, args);
+  if (!new_item)
+    return 0;
+
+  /*
+    THD::change_item_tree() should be called only if the tree was
+    really transformed, i.e. when a new item has been created.
+    Otherwise we'll be allocating a lot of unnecessary memory for
+    change records at each execution.
+  */
+  if (arg != new_item)
+    current_thd->change_item_tree(&arg, new_item);
+  return (this->*transformer)(args);
 }
 
 

--- 1.205/sql/item.h	2006-08-24 15:49:20 +04:00
+++ 1.206/sql/item.h	2006-08-24 15:49:20 +04:00
@@ -734,10 +734,7 @@ public:
     return (this->*processor)(arg);
   }
 
-  virtual Item* transform(Item_transformer transformer, byte *arg)
-  {
-    return (this->*transformer)(arg);
-  }
+  virtual Item* transform(Item_transformer transformer, byte *arg);
 
    virtual void traverse_cond(Cond_traverser traverser,
                               void *arg, traverse_order order)
@@ -755,7 +752,7 @@ public:
   virtual bool is_expensive_processor(byte *arg) { return 0; }
 
   virtual Item *equal_fields_propagator(byte * arg) { return this; }
-  virtual Item *set_no_const_sub(byte *arg) { return this; }
+  virtual bool set_no_const_sub(byte *arg) { return FALSE; }
   virtual Item *replace_equal_field(byte * arg) { return this; }
 
   /*
@@ -1255,7 +1252,7 @@ public:
   }
   Item_equal *find_item_equal(COND_EQUAL *cond_equal);
   Item *equal_fields_propagator(byte *arg);
-  Item *set_no_const_sub(byte *arg);
+  bool set_no_const_sub(byte *arg);
   Item *replace_equal_field(byte *arg);
   inline uint32 max_disp_length() { return field->max_length(); }
   Item_field *filed_for_view_update() { return this; }
@@ -2116,18 +2113,7 @@ public:
       (this->*processor)(args);
   }
 
-  /* 
-     This method like the walk method traverses the item tree, but
-     at the same time it can replace some nodes in the tree
-  */ 
-  Item *transform(Item_transformer transformer, byte *args)
-  {
-    Item *new_item= arg->transform(transformer, args);
-    if (!new_item)
-      return 0;
-    arg= new_item;
-    return (this->*transformer)(args);
-  }
+  Item *transform(Item_transformer transformer, byte *args);
 };
 
 /*

--- 1.213/sql/item_cmpfunc.cc	2006-08-24 15:49:20 +04:00
+++ 1.214/sql/item_cmpfunc.cc	2006-08-24 15:49:20 +04:00
@@ -500,8 +500,8 @@ int Arg_comparator::set_compare_func(Ite
         which would be transformed to:
         WHERE col= 'j'
       */
-      (*a)->transform(&Item::set_no_const_sub, (byte*) 0);
-      (*b)->transform(&Item::set_no_const_sub, (byte*) 0);
+      (*a)->walk(&Item::set_no_const_sub, (byte*) 0);
+      (*b)->walk(&Item::set_no_const_sub, (byte*) 0);
     }
     break;
   }
@@ -2753,6 +2753,8 @@ bool Item_cond::walk(Item_processor proc
 
 Item *Item_cond::transform(Item_transformer transformer, byte *arg)
 {
+  DBUG_ASSERT(!current_thd->is_stmt_prepare());
+
   List_iterator<Item> li(list);
   Item *item;
   while ((item= li++))
@@ -2760,8 +2762,15 @@ Item *Item_cond::transform(Item_transfor
     Item *new_item= item->transform(transformer, arg);
     if (!new_item)
       return 0;
+
+    /*
+      THD::change_item_tree() should be called only if the tree was
+      really transformed, i.e. when a new item has been created.
+      Otherwise we'll be allocating a lot of unnecessary memory for
+      change records at each execution.
+    */
     if (new_item != item)
-      li.replace(new_item);
+      current_thd->change_item_tree(li.ref(), new_item);
   }
   return Item_func::transform(transformer, arg);
 }
@@ -4006,6 +4015,8 @@ bool Item_equal::walk(Item_processor pro
 
 Item *Item_equal::transform(Item_transformer transformer, byte *arg)
 {
+  DBUG_ASSERT(!current_thd->is_stmt_prepare());
+
   List_iterator<Item_field> it(fields);
   Item *item;
   while ((item= it++))
@@ -4013,8 +4024,15 @@ Item *Item_equal::transform(Item_transfo
     Item *new_item= item->transform(transformer, arg);
     if (!new_item)
       return 0;
+
+    /*
+      THD::change_item_tree() should be called only if the tree was
+      really transformed, i.e. when a new item has been created.
+      Otherwise we'll be allocating a lot of unnecessary memory for
+      change records at each execution.
+    */
     if (new_item != item)
-      it.replace((Item_field *) new_item);
+      current_thd->change_item_tree((Item **) it.ref(), new_item);
   }
   return Item_func::transform(transformer, arg);
 }

--- 1.295/sql/item_func.cc	2006-08-24 15:49:20 +04:00
+++ 1.296/sql/item_func.cc	2006-08-24 15:49:20 +04:00
@@ -258,6 +258,8 @@ void Item_func::traverse_cond(Cond_trave
 
 Item *Item_func::transform(Item_transformer transformer, byte *argument)
 {
+  DBUG_ASSERT(!current_thd->is_stmt_prepare());
+
   if (arg_count)
   {
     Item **arg,**arg_end;
@@ -266,6 +268,13 @@ Item *Item_func::transform(Item_transfor
       Item *new_item= (*arg)->transform(transformer, argument);
       if (!new_item)
 	return 0;
+
+      /*
+        THD::change_item_tree() should be called only if the tree was
+        really transformed, i.e. when a new item has been created.
+        Otherwise we'll be allocating a lot of unnecessary memory for
+        change records at each execution.
+      */
       if (*arg != new_item)
         current_thd->change_item_tree(arg, new_item);
     }

--- 1.279/sql/item_strfunc.cc	2006-08-24 15:49:20 +04:00
+++ 1.280/sql/item_strfunc.cc	2006-08-24 15:49:20 +04:00
@@ -2051,6 +2051,26 @@ String *Item_func_make_set::val_str(Stri
 }
 
 
+Item *Item_func_make_set::transform(Item_transformer transformer, byte *arg)
+{
+  DBUG_ASSERT(!current_thd->is_stmt_prepare());
+
+  Item *new_item= item->transform(transformer, arg);
+  if (!new_item)
+    return 0;
+
+  /*
+    THD::change_item_tree() should be called only if the tree was
+    really transformed, i.e. when a new item has been created.
+    Otherwise we'll be allocating a lot of unnecessary memory for
+    change records at each execution.
+  */
+  if (item != new_item)
+    current_thd->change_item_tree(&item, new_item);
+  return Item_str_func::transform(transformer, arg);
+}
+
+
 void Item_func_make_set::print(String *str)
 {
   str->append(STRING_WITH_LEN("make_set("));

--- 1.112/sql/item_strfunc.h	2006-08-24 15:49:20 +04:00
+++ 1.113/sql/item_strfunc.h	2006-08-24 15:49:20 +04:00
@@ -475,14 +475,7 @@ public:
     return item->walk(processor, arg) ||
       Item_str_func::walk(processor, arg);
   }
-  Item *transform(Item_transformer transformer, byte *arg)
-  {
-    Item *new_item= item->transform(transformer, arg);
-    if (!new_item)
-      return 0;
-    item= new_item;
-    return Item_str_func::transform(transformer, arg);
-  }
+  Item *transform(Item_transformer transformer, byte *arg);
   void print(String *str);
 };
 

--- 1.32/sql/item_row.cc	2006-08-24 15:49:20 +04:00
+++ 1.33/sql/item_row.cc	2006-08-24 15:49:20 +04:00
@@ -154,12 +154,22 @@ bool Item_row::walk(Item_processor proce
 
 Item *Item_row::transform(Item_transformer transformer, byte *arg)
 {
+  DBUG_ASSERT(!current_thd->is_stmt_prepare());
+
   for (uint i= 0; i < arg_count; i++)
   {
     Item *new_item= items[i]->transform(transformer, arg);
     if (!new_item)
       return 0;
-    items[i]= new_item;
+
+    /*
+      THD::change_item_tree() should be called only if the tree was
+      really transformed, i.e. when a new item has been created.
+      Otherwise we'll be allocating a lot of unnecessary memory for
+      change records at each execution.
+    */
+    if (items[i] != new_item)
+      current_thd->change_item_tree(&items[i], new_item);
   }
   return (this->*transformer)(arg);
 }

--- 1.71/mysql-test/r/ps.result	2006-08-24 15:49:20 +04:00
+++ 1.72/mysql-test/r/ps.result	2006-08-24 15:49:20 +04:00
@@ -1297,3 +1297,18 @@ ERROR 3D000: No database selected
 create temporary table t1 (i int);
 ERROR 3D000: No database selected
 use test;
+DROP TABLE IF EXISTS t1, t2, t3;
+CREATE TABLE t1 (i BIGINT, j BIGINT);
+CREATE TABLE t2 (i BIGINT);
+CREATE TABLE t3 (i BIGINT, j BIGINT);
+PREPARE stmt FROM "SELECT * FROM t1 JOIN t2 ON (t2.i = t1.i)
+                   LEFT JOIN t3 ON ((t3.i, t3.j) = (t1.i, t1.j))
+                   WHERE t1.i = ?";
+SET @a= 1;
+EXECUTE stmt USING @a;
+i	j	i	i	j
+EXECUTE stmt USING @a;
+i	j	i	i	j
+DEALLOCATE PREPARE stmt;
+DROP TABLE IF EXISTS t1, t2, t3;
+End of 5.0 tests.

--- 1.68/mysql-test/t/ps.test	2006-08-24 15:49:20 +04:00
+++ 1.69/mysql-test/t/ps.test	2006-08-24 15:49:20 +04:00
@@ -1329,4 +1329,33 @@ create temporary table t1 (i int);
 # Restore the old environemnt
 #
 use test;
-# End of 5.0 tests
+
+
+#
+# BUG#21166: Prepared statement causes signal 11 on second execution
+#
+# Changes in an item tree done by optimizer weren't properly
+# registered and went unnoticed, which resulted in preliminary freeing
+# of used memory.
+#
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2, t3;
+--enable_warnings
+
+CREATE TABLE t1 (i BIGINT, j BIGINT);
+CREATE TABLE t2 (i BIGINT);
+CREATE TABLE t3 (i BIGINT, j BIGINT);
+
+PREPARE stmt FROM "SELECT * FROM t1 JOIN t2 ON (t2.i = t1.i)
+                   LEFT JOIN t3 ON ((t3.i, t3.j) = (t1.i, t1.j))
+                   WHERE t1.i = ?";
+
+SET @a= 1;
+EXECUTE stmt USING @a;
+EXECUTE stmt USING @a;
+
+DEALLOCATE PREPARE stmt;
+DROP TABLE IF EXISTS t1, t2, t3;
+
+
+--echo End of 5.0 tests.