bzr commit into mysql-5.1 branch (kgeorge:2662) Bug#34773

[Georgi Kodinov]

#At file:///home/kgeorge/mysql/bzr/B34773-5.1-bugteam/

 2662 Georgi Kodinov	2008-06-12
      Bug#34773: query with explain extended and derived table / other table 
        crashes server
      
      When creating temporary table that contains aggregate functions a 
      non-reversible source transformation was performed to redirect aggregate
      function arguments towards temporary table columns.
      This caused EXPLAIN EXTENDED to fail because it was trying to resolve
      references to the (freed) temporary table.
      Fixed by preserving the original aggregate function arguments and
      using them (instead of the transformed ones) for EXPLAIN EXTENDED.
modified:
  mysql-test/r/explain.result
  mysql-test/t/explain.test
  sql/item.cc
  sql/item_sum.cc
  sql/item_sum.h
  sql/opt_range.cc
  sql/opt_sum.cc
  sql/sql_select.cc

per-file messages:
  mysql-test/r/explain.result
    Bug#34773: test case
  mysql-test/t/explain.test
    Bug#34773: test case
  sql/item.cc
    Bug#34773: use accessor functions instead of public members
  sql/item_sum.cc
    Bug#34773: 
     - Encapsulate the arguments into Item_sum and
    provide accessor and mutator methods 
     - print the orginal arguments (if present)
       in EXPLAIN EXTENDED
  sql/item_sum.h
    Bug#34773: Encapsulate the arguments into Item_sum and
    provide accessor and mutator methods.
  sql/opt_range.cc
    Bug#34773: use accessor functions instead of public members
  sql/opt_sum.cc
    Bug#34773: use accessor functions instead of public members
  sql/sql_select.cc
    Bug#34773: use accessor functions instead of public members
=== modified file 'mysql-test/r/explain.result'
--- a/mysql-test/r/explain.result	2007-11-16 11:00:57 +0000
+++ b/mysql-test/r/explain.result	2008-06-12 13:23:09 +0000
@@ -107,3 +107,24 @@ X	X	X	X	X	X	X	X	X	
 X	X	X	X	X	X	X	X	X	Range checked for each record (index map: 0xFFFFFFFFFF)
 DROP TABLE t2;
 DROP TABLE t1;
+CREATE TABLE t1(a INT);
+CREATE TABLE t2(a INT);
+INSERT INTO t1 VALUES (1),(2);
+INSERT INTO t2 VALUES (1),(2);
+EXPLAIN EXTENDED SELECT 1
+FROM (SELECT COUNT(DISTINCT t1.a) FROM t1,t2 GROUP BY t1.a) AS s1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	PRIMARY	<derived2>	ALL	NULL	NULL	NULL	NULL	2	100.00	
+2	DERIVED	t1	ALL	NULL	NULL	NULL	NULL	2	100.00	Using temporary; Using filesort
+2	DERIVED	t2	ALL	NULL	NULL	NULL	NULL	2	100.00	Using join buffer
+Warnings:
+Note	1003	select 1 AS `1` from (select count(distinct `test`.`t1`.`a`) AS `COUNT(DISTINCT
t1.a)` from `test`.`t1` join `test`.`t2` group by `test`.`t1`.`a`) `s1`
+EXPLAIN EXTENDED SELECT 1
+FROM (SELECT COUNT(DISTINCT t1.a) FROM t1,t2 GROUP BY t1.a) AS s1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	PRIMARY	<derived2>	ALL	NULL	NULL	NULL	NULL	2	100.00	
+2	DERIVED	t1	ALL	NULL	NULL	NULL	NULL	2	100.00	Using temporary; Using filesort
+2	DERIVED	t2	ALL	NULL	NULL	NULL	NULL	2	100.00	Using join buffer
+Warnings:
+Note	1003	select 1 AS `1` from (select count(distinct `test`.`t1`.`a`) AS `COUNT(DISTINCT
t1.a)` from `test`.`t1` join `test`.`t2` group by `test`.`t1`.`a`) `s1`
+DROP TABLE t1,t2;

=== modified file 'mysql-test/t/explain.test'
--- a/mysql-test/t/explain.test	2007-11-16 11:00:57 +0000
+++ b/mysql-test/t/explain.test	2008-06-12 13:23:09 +0000
@@ -94,4 +94,22 @@ EXPLAIN SELECT 1 FROM
 DROP TABLE t2;
 DROP TABLE t1;
 
+#
+# Bug #34773: query with explain extended and derived table / other table
+# crashes server
+#
+
+CREATE TABLE t1(a INT);
+CREATE TABLE t2(a INT);
+INSERT INTO t1 VALUES (1),(2);
+INSERT INTO t2 VALUES (1),(2);
+
+EXPLAIN EXTENDED SELECT 1
+ FROM (SELECT COUNT(DISTINCT t1.a) FROM t1,t2 GROUP BY t1.a) AS s1;
+
+EXPLAIN EXTENDED SELECT 1
+ FROM (SELECT COUNT(DISTINCT t1.a) FROM t1,t2 GROUP BY t1.a) AS s1;
+
+DROP TABLE t1,t2;
+
 # End of 5.0 tests.

=== modified file 'sql/item.cc'
--- a/sql/item.cc	2008-05-20 07:38:17 +0000
+++ b/sql/item.cc	2008-06-12 13:23:09 +0000
@@ -6919,7 +6919,7 @@ enum_field_types Item_type_holder::get_r
     */
     Item_sum *item_sum= (Item_sum *) item;
     if (item_sum->keep_field_type())
-      return get_real_type(item_sum->args[0]);
+      return get_real_type(item_sum->get_arg(0));
     break;
   }
   case FUNC_ITEM:
@@ -7182,7 +7182,7 @@ void Item_type_holder::get_full_info(Ite
     if (item->type() == Item::SUM_FUNC_ITEM &&
         (((Item_sum*)item)->sum_func() == Item_sum::MAX_FUNC ||
          ((Item_sum*)item)->sum_func() == Item_sum::MIN_FUNC))
-      item = ((Item_sum*)item)->args[0];
+      item = ((Item_sum*)item)->get_arg(0);
     /*
       We can have enum/set type after merging only if we have one enum|set
       field (or MIN|MAX(enum|set field)) and number of NULL fields

=== modified file 'sql/item_sum.cc'
--- a/sql/item_sum.cc	2008-05-01 11:54:59 +0000
+++ b/sql/item_sum.cc	2008-06-12 13:23:09 +0000
@@ -356,7 +356,7 @@ bool Item_sum::register_sum_func(THD *th
 
 
 Item_sum::Item_sum(List<Item> &list) :arg_count(list.elements), 
-  forced_const(FALSE)
+  orig_args(NULL), forced_const(FALSE)
 {
   if ((args=(Item**) sql_alloc(sizeof(Item*)*arg_count)))
   {
@@ -379,10 +379,12 @@ Item_sum::Item_sum(List<Item> &list) :ar
 */
 
 Item_sum::Item_sum(THD *thd, Item_sum *item):
-  Item_result_field(thd, item), arg_count(item->arg_count),
+  Item_result_field(thd, item),
   aggr_sel(item->aggr_sel),
   nest_level(item->nest_level), aggr_level(item->aggr_level),
-  quick_group(item->quick_group), used_tables_cache(item->used_tables_cache),
+  quick_group(item->quick_group),
+  arg_count(item->arg_count), orig_args(NULL),
+  used_tables_cache(item->used_tables_cache),
   forced_const(item->forced_const) 
 {
   if (arg_count <= 2)
@@ -425,12 +427,13 @@ void Item_sum::make_field(Send_field *tm
 
 void Item_sum::print(String *str, enum_query_type query_type)
 {
+  Item **pargs= orig_args ? orig_args : args;
   str->append(func_name());
   for (uint i=0 ; i < arg_count ; i++)
   {
     if (i)
       str->append(',');
-    args[i]->print(str, query_type);
+    pargs[i]->print(str, query_type);
   }
   str->append(')');
 }
@@ -535,6 +538,18 @@ void Item_sum::update_used_tables ()
 }
 
 
+Item *Item_sum::set_arg(int i, THD *thd, Item *new_val) 
+{
+  if (!orig_args)
+  {
+    orig_args= (Item **) sql_alloc (sizeof (Item *) * arg_count);
+    memcpy (orig_args, args, sizeof (Item *) * arg_count);
+  }
+  thd->change_item_tree(args + i, new_val);
+  return new_val;
+}
+
+
 String *
 Item_sum_num::val_str(String *str)
 {

=== modified file 'sql/item_sum.h'
--- a/sql/item_sum.h	2008-03-28 15:09:14 +0000
+++ b/sql/item_sum.h	2008-06-12 13:23:09 +0000
@@ -228,10 +228,8 @@ public:
     VARIANCE_FUNC, SUM_BIT_FUNC, UDF_SUM_FUNC, GROUP_CONCAT_FUNC
   };
 
-  Item **args, *tmp_args[2];
   Item **ref_by; /* pointer to a ref to the object used to register it */
   Item_sum *next; /* next in the circular chain of registered objects  */
-  uint arg_count;
   Item_sum *in_sum_func;  /* embedding set function if any */ 
   st_select_lex * aggr_sel; /* select where the function is aggregated       */ 
   int8 nest_level;        /* number of the nesting level of the set function */
@@ -248,24 +246,32 @@ public:
   List<Item_field> outer_fields;
 
 protected:  
+  uint arg_count;
+  Item **args, *tmp_args[2];
+  /* 
+    Copy of args (for EXPLAIN EXTENDED) when source transformations on them 
+    is needed.
+  */
+  Item **orig_args;
   table_map used_tables_cache;
   bool forced_const;
 
 public:  
 
   void mark_as_sum_func();
-  Item_sum() :arg_count(0), quick_group(1), forced_const(FALSE)
+  Item_sum() :quick_group(1), arg_count(0), orig_args(NULL), 
+    forced_const(FALSE)
   {
     mark_as_sum_func();
   }
-  Item_sum(Item *a) :args(tmp_args), arg_count(1), quick_group(1), 
-    forced_const(FALSE)
+  Item_sum(Item *a) :quick_group(1), arg_count(1), args(tmp_args),
+    orig_args(NULL), forced_const(FALSE)
   {
     args[0]=a;
     mark_as_sum_func();
   }
-  Item_sum( Item *a, Item *b ) :args(tmp_args), arg_count(2), quick_group(1),
-    forced_const(FALSE)
+  Item_sum( Item *a, Item *b ) :quick_group(1), arg_count(2), args(tmp_args),
+    orig_args(NULL), forced_const(FALSE)
   {
     args[0]=a; args[1]=b;
     mark_as_sum_func();
@@ -374,6 +380,10 @@ public:  
   bool register_sum_func(THD *thd, Item **ref);
   st_select_lex *depended_from() 
     { return (nest_level == aggr_level ? 0 : aggr_sel); }
+
+  Item *get_arg(int i) { return args[i]; }
+  Item *set_arg(int i, THD *thd, Item *new_val);
+  uint get_arg_count() { return arg_count; }
 };
 
 

=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc	2008-04-14 10:58:53 +0000
+++ b/sql/opt_range.cc	2008-06-12 13:23:09 +0000
@@ -9157,7 +9157,7 @@ get_best_group_min_max(PARAM *param, SEL
         DBUG_RETURN(NULL);
 
       /* The argument of MIN/MAX. */
-      Item *expr= min_max_item->args[0]->real_item();    
+      Item *expr= min_max_item->get_arg(0)->real_item();
       if (expr->type() == Item::FIELD_ITEM) /* Is it an attribute? */
       {
         if (! min_max_arg_item)

=== modified file 'sql/opt_sum.cc'
--- a/sql/opt_sum.cc	2007-12-20 21:11:37 +0000
+++ b/sql/opt_sum.cc	2008-06-12 13:23:09 +0000
@@ -199,7 +199,7 @@ int opt_sum_query(TABLE_LIST *tables, Li
           to the number of rows in the tables if this number is exact and
           there are no outer joins.
         */
-        if (!conds && !((Item_sum_count*) item)->args[0]->maybe_null
&&
+        if (!conds && !((Item_sum_count*) item)->get_arg(0)->maybe_null
&&
             !outer_tables && maybe_exact_count)
         {
           if (!is_exact_count)
@@ -225,7 +225,7 @@ int opt_sum_query(TABLE_LIST *tables, Li
           parts of the key is found in the COND, then we can use
           indexes to find the key.
         */
-        Item *expr=item_sum->args[0];
+        Item *expr=item_sum->get_arg(0);
         if (expr->real_item()->type() == Item::FIELD_ITEM)
         {
           uchar key_buff[MAX_KEY_LENGTH];
@@ -373,7 +373,7 @@ int opt_sum_query(TABLE_LIST *tables, Li
           parts of the key is found in the COND, then we can use
           indexes to find the key.
         */
-        Item *expr=item_sum->args[0];
+        Item *expr=item_sum->get_arg(0);
         if (expr->real_item()->type() == Item::FIELD_ITEM)
         {
           uchar key_buff[MAX_KEY_LENGTH];

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2008-05-16 16:03:50 +0000
+++ b/sql/sql_select.cc	2008-06-12 13:23:09 +0000
@@ -9780,11 +9780,11 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
     }
     if (type == Item::SUM_FUNC_ITEM && !group && !save_sum_fields)
     {						/* Can't calc group yet */
-      ((Item_sum*) item)->result_field=0;
-      for (i=0 ; i < ((Item_sum*) item)->arg_count ; i++)
+      Item_sum *sum_item= (Item_sum *) item;
+      sum_item->result_field=0;
+      for (i=0 ; i < sum_item->get_arg_count() ; i++)
       {
-	Item **argp= ((Item_sum*) item)->args + i;
-	Item *arg= *argp;
+	Item *arg= sum_item->get_arg(i);
 	if (!arg->const_item())
 	{
 	  Field *new_field=
@@ -9812,7 +9812,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
             string_total_length+= new_field->pack_length();
           }
           thd->mem_root= mem_root_save;
-          thd->change_item_tree(argp, new Item_field(new_field));
+          arg= sum_item->set_arg(i, thd, new Item_field(new_field));
           thd->mem_root= &table->mem_root;
 	  if (!(new_field->flags & NOT_NULL_FLAG))
           {
@@ -9821,7 +9821,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
               new_field->maybe_null() is still false, it will be
               changed below. But we have to setup Item_field correctly
             */
-            (*argp)->maybe_null=1;
+            arg->maybe_null=1;
           }
           new_field->field_index= fieldnr++;
 	}
@@ -14491,9 +14491,9 @@ count_field_types(SELECT_LEX *select_lex
             param->quick_group=0;			// UDF SUM function
           param->sum_func_count++;
 
-          for (uint i=0 ; i < sum_item->arg_count ; i++)
+          for (uint i=0 ; i < sum_item->get_arg_count() ; i++)
           {
-            if (sum_item->args[0]->real_item()->type() == Item::FIELD_ITEM)
+            if (sum_item->get_arg(i)->real_item()->type() == Item::FIELD_ITEM)
               param->field_count++;
             else
               param->func_count++;