Merge bk-internal.mysql.com:/home/bk/mysql-4.1

into mysql.com:/media/sda1/mysql/mysql-4.1-5748-anew

mysql-test/r/ps.result

@@ -297,3 +297,14 @@ execute stmt;
 'abc' like convert('abc' using utf8)
 1
 deallocate prepare stmt;
+create table t1 ( a bigint );
+prepare stmt from 'select a from t1 where a between ? and ?';
+set @a=1;
+execute stmt using @a, @a;
+a
+execute stmt using @a, @a;
+a
+execute stmt using @a, @a;
+a
+drop table t1;
+deallocate prepare stmt;

mysql-test/t/ps.test

@@ -314,3 +314,18 @@ prepare stmt from "select 'abc' like convert('abc' using utf8)";
 execute stmt;
 execute stmt;
 deallocate prepare stmt;
+
+# 
+# BUG#5748 "Prepared statement with BETWEEN and bigint values crashes
+# mysqld". Just another place where an item tree modification must be 
+# rolled back.
+# 
+create table t1 ( a bigint );
+prepare stmt from 'select a from t1 where a between ? and ?';
+set @a=1;
+execute stmt using @a, @a;
+execute stmt using @a, @a;
+execute stmt using @a, @a;
+drop table t1;
+deallocate prepare stmt;
+

sql/item.cc

@@ -106,7 +106,7 @@ void Item::print_item_w_name(String *str)
 Item_ident::Item_ident(const char *db_name_par,const char *table_name_par,
 		       const char *field_name_par)
   :orig_db_name(db_name_par), orig_table_name(table_name_par), 
-   orig_field_name(field_name_par), changed_during_fix_field(0), 
+   orig_field_name(field_name_par),
    db_name(db_name_par), table_name(table_name_par), 
    field_name(field_name_par), cached_field_index(NO_CACHED_FIELD_INDEX), 
    cached_table(0), depended_from(0)
@@ -120,7 +120,6 @@ Item_ident::Item_ident(THD *thd, Item_ident *item)
    orig_db_name(item->orig_db_name),
    orig_table_name(item->orig_table_name), 
    orig_field_name(item->orig_field_name),
-   changed_during_fix_field(0),
    db_name(item->db_name),
    table_name(item->table_name),
    field_name(item->field_name),
@@ -137,11 +136,6 @@ void Item_ident::cleanup()
 		       table_name, orig_table_name,
 		       field_name, orig_field_name));
   Item::cleanup();
-  if (changed_during_fix_field)
-  {
-    *changed_during_fix_field= this;
-    changed_during_fix_field= 0;
-  }
   db_name= orig_db_name; 
   table_name= orig_table_name;
   field_name= orig_field_name;
@@ -1340,10 +1334,10 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 
 	Item_ref *rf;
 	*ref= rf= new Item_ref(last->ref_pointer_array + counter,
-			       ref,
+			       0,
 			       (char *)table_name,
 			       (char *)field_name);
-	register_item_tree_changing(ref);
+        thd->register_item_tree_change(ref, this, &thd->mem_root);
 	if (!rf)
 	  return 1;
 	/*
@@ -1362,7 +1356,8 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 	if (last->having_fix_field)
 	{
 	  Item_ref *rf;
-	  *ref= rf= new Item_ref(ref, *ref,
+          thd->register_item_tree_change(ref, *ref, &thd->mem_root);
+	  *ref= rf= new Item_ref(ref, 0,
 				 (where->db[0]?where->db:0), 
 				 (char *)where->alias,
 				 (char *)field_name);
@@ -2003,7 +1998,7 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 	Item_field* fld;
 	if (!((*reference)= fld= new Item_field(tmp)))
 	  return 1;
-	register_item_tree_changing(reference);
+	thd->register_item_tree_change(reference, this, &thd->mem_root);
 	mark_as_dependent(thd, last, thd->lex->current_select, fld);
 	return 0;
       }

sql/item.h

@@ -317,7 +317,6 @@ class Item_ident :public Item
   const char *orig_db_name;
   const char *orig_table_name;
   const char *orig_field_name;
-  Item **changed_during_fix_field;
 public:
   const char *db_name;
   const char *table_name;
@@ -340,8 +339,6 @@ public:
   Item_ident(THD *thd, Item_ident *item);
   const char *full_name() const;
   void cleanup();
-  void register_item_tree_changing(Item **ref)
-    { changed_during_fix_field= ref; }
   bool remove_dependence_processor(byte * arg);
 };
 

sql/item_cmpfunc.cc

@@ -145,7 +145,7 @@ void Item_func_not_all::print(String *str)
   1	Item was replaced with an integer version of the item
 */
 
-static bool convert_constant_item(Field *field, Item **item)
+static bool convert_constant_item(THD *thd, Field *field, Item **item)
 {
   if ((*item)->const_item())
   {
@@ -153,7 +153,10 @@ static bool convert_constant_item(Field *field, Item **item)
     {
       Item *tmp=new Item_int_with_ref(field->val_int(), *item);
       if (tmp)
+      {
+        thd->register_item_tree_change(item, *item, &thd->mem_root);
 	*item=tmp;
+      }
       return 1;					// Item was replaced
     }
   }
@@ -164,6 +167,7 @@ static bool convert_constant_item(Field *field, Item **item)
 void Item_bool_func2::fix_length_and_dec()
 {
   max_length= 1;				     // Function returns 0 or 1
+  THD *thd= current_thd;
 
   /*
     As some compare functions are generated after sql_yacc,
@@ -199,7 +203,6 @@ void Item_bool_func2::fix_length_and_dec()
         !coll.set(args[0]->collation, args[1]->collation, TRUE))
     {
       Item* conv= 0;
-      THD *thd= current_thd;
       Item_arena *arena= thd->current_arena, backup;
       strong= coll.strong;
       weak= strong ? 0 : 1;
@@ -245,7 +248,7 @@ void Item_bool_func2::fix_length_and_dec()
     Field *field=((Item_field*) args[0])->field;
     if (field->can_be_compared_as_longlong())
     {
-      if (convert_constant_item(field,&args[1]))
+      if (convert_constant_item(thd, field,&args[1]))
       {
 	cmp.set_cmp_func(this, tmp_arg, tmp_arg+1,
 			 INT_RESULT);		// Works for all types.
@@ -258,7 +261,7 @@ void Item_bool_func2::fix_length_and_dec()
     Field *field=((Item_field*) args[1])->field;
     if (field->can_be_compared_as_longlong())
     {
-      if (convert_constant_item(field,&args[0]))
+      if (convert_constant_item(thd, field,&args[0]))
       {
 	cmp.set_cmp_func(this, tmp_arg, tmp_arg+1,
 			 INT_RESULT); // Works for all types.
@@ -836,6 +839,7 @@ longlong Item_func_interval::val_int()
 void Item_func_between::fix_length_and_dec()
 {
    max_length= 1;
+   THD *thd= current_thd;
 
   /*
     As some compare functions are generated after sql_yacc,
@@ -858,9 +862,9 @@ void Item_func_between::fix_length_and_dec()
     Field *field=((Item_field*) args[0])->field;
     if (field->can_be_compared_as_longlong())
     {
-      if (convert_constant_item(field,&args[1]))
+      if (convert_constant_item(thd, field,&args[1]))
 	cmp_type=INT_RESULT;			// Works for all types.
-      if (convert_constant_item(field,&args[2]))
+      if (convert_constant_item(thd, field,&args[2]))
 	cmp_type=INT_RESULT;			// Works for all types.
     }
   }

sql/sql_class.cc

@@ -697,6 +697,54 @@ void THD::close_active_vio()
 #endif
 
 
+struct Item_change_record: public ilink
+{
+  Item **place;
+  Item *old_value;
+  /* Placement new was hidden by `new' in ilink (TODO: check): */
+  static void *operator new(unsigned int size, void *mem) { return mem; }
+};
+
+
+/*
+  Register an item tree tree transformation, performed by the query
+  optimizer. We need a pointer to runtime_memroot because it may be !=
+  thd->mem_root (due to possible set_n_backup_item_arena called for thd).
+*/
+
+void THD::nocheck_register_item_tree_change(Item **place, Item *old_value,
+                                            MEM_ROOT *runtime_memroot)
+{
+  Item_change_record *change;
+  /*
+    Now we use one node per change, which adds some memory overhead,
+    but still is rather fast as we use alloc_root for allocations.
+    A list of item tree changes of an average query should be short.
+  */
+  void *change_mem= alloc_root(runtime_memroot, sizeof(*change));
+  if (change_mem == 0)
+  {
+    fatal_error();
+    return;
+  }
+  change= new (change_mem) Item_change_record;
+  change->place= place;
+  change->old_value= old_value;
+  change_list.push_back(change);
+}
+
+
+void THD::rollback_item_tree_changes()
+{
+  I_List_iterator<Item_change_record> it(change_list);
+  Item_change_record *change;
+  while ((change= it++))
+    *change->place= change->old_value;
+  /* We can forget about changes memory: it's allocated in runtime memroot */
+  change_list.empty();
+}
+
+
 /*****************************************************************************
 ** Functions to provide a interface to select results
 *****************************************************************************/

sql/sql_class.h

@@ -461,6 +461,8 @@ public:
 
   inline bool is_stmt_prepare() const { return (int)state < (int)PREPARED; }
   inline bool is_first_stmt_execute() const { return state == PREPARED; }
+  inline bool is_conventional_execution() const
+  { return state == CONVENTIONAL_EXECUTION; }
   inline gptr alloc(unsigned int size) { return alloc_root(&mem_root,size); }
   inline gptr calloc(unsigned int size)
   {
@@ -640,6 +642,17 @@ private:
 };
 
 
+/*
+  A registry for item tree transformations performed during
+  query optimization. We register only those changes which require
+  a rollback to re-execute a prepared statement or stored procedure
+  yet another time.
+*/
+
+struct Item_change_record;
+typedef I_List<Item_change_record> Item_change_list;
+
+
 /*
   For each client connection we create a separate thread with THD serving as
   a thread/connection descriptor
@@ -808,6 +821,14 @@ public:
 #ifdef SIGNAL_WITH_VIO_CLOSE
   Vio* active_vio;
 #endif
+  /*
+    This is to track items changed during execution of a prepared
+    statement/stored procedure. It's created by
+    register_item_tree_change() in memory root of THD, and freed in
+    rollback_item_tree_changes(). For conventional execution it's always 0.
+  */
+  Item_change_list change_list;
+
   /*
     Current prepared Item_arena if there one, or 0
   */
@@ -1032,6 +1053,16 @@ public:
   }
   inline CHARSET_INFO *charset() { return variables.character_set_client; }
   void update_charset();
+
+  void register_item_tree_change(Item **place, Item *old_value,
+                                 MEM_ROOT *runtime_memroot)
+  {
+    if (!current_arena->is_conventional_execution())
+      nocheck_register_item_tree_change(place, old_value, runtime_memroot);
+  }
+  void nocheck_register_item_tree_change(Item **place, Item *old_value,
+                                             MEM_ROOT *runtime_memroot);
+  void rollback_item_tree_changes();
 };
 
 /* Flags for the THD::system_thread (bitmap) variable */

sql/sql_parse.cc

@@ -4056,6 +4056,7 @@ void mysql_parse(THD *thd, char *inBuf, uint length)
     }
     thd->proc_info="freeing items";
     thd->end_statement();
+    DBUG_ASSERT(thd->change_list.is_empty());
   }
   DBUG_VOID_RETURN;
 }

sql/sql_prepare.cc

@@ -1614,6 +1614,7 @@ int mysql_stmt_prepare(THD *thd, char *packet, uint packet_length,
   cleanup_items(stmt->free_list);
   close_thread_tables(thd);
   free_items(thd->free_list);
+  thd->rollback_item_tree_changes();
   thd->free_list= 0;
   thd->current_arena= thd;
 
@@ -1870,8 +1871,7 @@ static void execute_stmt(THD *thd, Prepared_statement *stmt,
     transformations of the query tree (i.e. negations elimination).
     This should be done permanently on the parse tree of this statement.
   */
-  if (stmt->state == Item_arena::PREPARED)
-    thd->current_arena= stmt;
+  thd->current_arena= stmt;
 
   if (!(specialflag & SPECIAL_NO_PRIOR))
     my_pthread_setprio(pthread_self(),QUERY_PRIOR);
@@ -1884,11 +1884,10 @@ static void execute_stmt(THD *thd, Prepared_statement *stmt,
   free_items(thd->free_list);
   thd->free_list= 0;
   if (stmt->state == Item_arena::PREPARED)
-  {
-    thd->current_arena= thd;
     stmt->state= Item_arena::EXECUTED;
-  }
+  thd->current_arena= thd;
   cleanup_items(stmt->free_list);
+  thd->rollback_item_tree_changes();
   reset_stmt_params(stmt);
   close_thread_tables(thd);                    // to close derived tables
   thd->set_statement(&thd->stmt_backup);

tests/client_test.c

@@ -8442,7 +8442,7 @@ static void test_subqueries_ref()
   int rc, i;
   const char *query= "SELECT a as ccc from t1 where a+1=(SELECT 1+ccc from t1 where ccc+1=a+1 and a=1)";
 
-  myheader("test_subquery_ref");
+  myheader("test_subqueries_ref");
 
   rc= mysql_query(mysql, "DROP TABLE IF EXISTS t1");
   myquery(rc);