bzr commit into mysql-5.1 branch (kgeorge:2757) Bug#37348

[Georgi Kodinov]

#At file:///home/kgeorge/mysql/bzr/B37348-5.1-5.1.29-rc/

 2757 Georgi Kodinov	2008-10-02
      Bug #37348: Crash in or immediately after JOIN::make_sum_func_list
      
      The optimizer pulls up aggregate functions which should be aggregated in
      an outer select. At some point it may substitute such a function for a field
      in the temporary table. The setup_copy_fields function doesn't take this
      into account and may overrun the copy_field buffer.
      
      Fixed by checking the type of the original item before considering an 
      Item_field referenced through an Item_aggregate_ref as a thing that 
      needs copying.
      Added an assertion to make sure bugs that cause similar discrepancy 
      don't go undetected.
modified:
  mysql-test/r/func_group.result
  mysql-test/t/func_group.test
  sql/item.cc
  sql/item.h
  sql/sql_select.cc

per-file messages:
  mysql-test/r/func_group.result
    Bug #37348: test case
  mysql-test/t/func_group.test
    Bug #37348: test case
  sql/item.cc
    Bug #37348: moved the definition of Item_aggregate_ref to the correct
    header file.
  sql/item.h
    Bug #37348: 
    - moved the definition of Item_aggregate_ref to the correct
      header file.
    - Added a member to collect the type of the item originally referenced 
      by this Item_ref subclass.     
    - Added a way to distinguish Item_aggregate_ref from the other types of refs
  sql/sql_select.cc
    Bug #37348: don't consider copying field references
    seen through Item_aggregate_ref that was not originally pointing to a 
    field.
=== modified file 'mysql-test/r/func_group.result'
--- a/mysql-test/r/func_group.result	2008-03-19 11:25:36 +0000
+++ b/mysql-test/r/func_group.result	2008-10-02 10:25:19 +0000
@@ -1416,4 +1416,41 @@ SELECT AVG(a), CAST(AVG(a) AS DECIMAL) F
 AVG(a)	CAST(AVG(a) AS DECIMAL)
 15	15
 DROP TABLE t1;
+CREATE TABLE derived1 (a bigint(21));
+INSERT INTO derived1 VALUES (2);
+CREATE TABLE D (
+pk int(11) NOT NULL AUTO_INCREMENT,
+int_nokey int(11) DEFAULT NULL,
+int_key int(11) DEFAULT NULL,
+filler blob,
+PRIMARY KEY (pk),
+KEY int_key (int_key)
+);
+INSERT INTO D VALUES 
+(39,40,4,repeat('  X', 42)),
+(43,56,4,repeat('  X', 42)),
+(47,12,4,repeat('  X', 42)),
+(71,28,4,repeat('  X', 42)),
+(76,54,4,repeat('  X', 42)),
+(83,45,4,repeat('  X', 42)),
+(105,53,12,NULL);
+SELECT 
+(SELECT COUNT( int_nokey ) 
+FROM derived1 AS X 
+WHERE 
+X.int_nokey < 61 
+GROUP BY pk 
+LIMIT 1) 
+FROM D AS X 
+WHERE X.int_key < 13  
+GROUP BY int_nokey LIMIT 1;
+(SELECT COUNT( int_nokey ) 
+FROM derived1 AS X 
+WHERE 
+X.int_nokey < 61 
+GROUP BY pk 
+LIMIT 1)
+1
+DROP TABLE derived1;
+DROP TABLE D;
 End of 5.0 tests

=== modified file 'mysql-test/t/func_group.test'
--- a/mysql-test/t/func_group.test	2008-03-19 11:25:36 +0000
+++ b/mysql-test/t/func_group.test	2008-10-02 10:25:19 +0000
@@ -933,5 +933,45 @@ SELECT AVG(a), CAST(AVG(a) AS DECIMAL) F
 
 DROP TABLE t1;
 
+#
+# Bug #37348: Crash in or immediately after JOIN::make_sum_func_list
+#
+
+CREATE TABLE derived1 (a bigint(21));
+INSERT INTO derived1 VALUES (2);
+
+
+CREATE TABLE D (
+  pk int(11) NOT NULL AUTO_INCREMENT,
+  int_nokey int(11) DEFAULT NULL,
+  int_key int(11) DEFAULT NULL,
+  filler blob,
+  PRIMARY KEY (pk),
+  KEY int_key (int_key)
+);
+
+INSERT INTO D VALUES 
+  (39,40,4,repeat('  X', 42)),
+  (43,56,4,repeat('  X', 42)),
+  (47,12,4,repeat('  X', 42)),
+  (71,28,4,repeat('  X', 42)),
+  (76,54,4,repeat('  X', 42)),
+  (83,45,4,repeat('  X', 42)),
+  (105,53,12,NULL);
+
+SELECT 
+  (SELECT COUNT( int_nokey ) 
+   FROM derived1 AS X 
+   WHERE 
+     X.int_nokey < 61 
+   GROUP BY pk 
+   LIMIT 1) 
+FROM D AS X 
+WHERE X.int_key < 13  
+GROUP BY int_nokey LIMIT 1;
+
+DROP TABLE derived1;
+DROP TABLE D;
+
 ###
 --echo End of 5.0 tests

=== modified file 'sql/item.cc'
--- a/sql/item.cc	2008-10-02 05:56:07 +0000
+++ b/sql/item.cc	2008-10-02 10:25:19 +0000
@@ -1319,28 +1319,6 @@ void Item_name_const::print(String *str,
 }
 
 
-/*
- need a special class to adjust printing : references to aggregate functions 
- must not be printed as refs because the aggregate functions that are added to
- the front of select list are not printed as well.
-*/
-class Item_aggregate_ref : public Item_ref
-{
-public:
-  Item_aggregate_ref(Name_resolution_context *context_arg, Item **item,
-                  const char *table_name_arg, const char *field_name_arg)
-    :Item_ref(context_arg, item, table_name_arg, field_name_arg) {}
-
-  virtual inline void print (String *str, enum_query_type query_type)
-  {
-    if (ref)
-      (*ref)->print(str, query_type);
-    else
-      Item_ident::print(str, query_type);
-  }
-};
-
-
 /**
   Move SUM items out from item tree and replace with reference.
 

=== modified file 'sql/item.h'
--- a/sql/item.h	2008-08-15 20:42:29 +0000
+++ b/sql/item.h	2008-10-02 10:25:19 +0000
@@ -2126,7 +2126,7 @@ class Item_ref :public Item_ident
 protected:
   void set_properties();
 public:
-  enum Ref_Type { REF, DIRECT_REF, VIEW_REF, OUTER_REF };
+  enum Ref_Type { REF, DIRECT_REF, VIEW_REF, OUTER_REF, AGGREGATE_REF };
   Field *result_field;			 /* Save result here */
   Item **ref;
   Item_ref(Name_resolution_context *context_arg,
@@ -2419,6 +2419,32 @@ public:
   virtual Item *real_item() { return ref; }
 };
 
+
+/*
+ need a special class to adjust printing : references to aggregate functions 
+ must not be printed as refs because the aggregate functions that are added to
+ the front of select list are not printed as well.
+*/
+class Item_aggregate_ref : public Item_ref
+{
+public:
+  Item::Type original_type;
+  Item_aggregate_ref(Name_resolution_context *context_arg, Item **item,
+                  const char *table_name_arg, const char *field_name_arg)
+    :Item_ref(context_arg, item, table_name_arg, field_name_arg), 
+     original_type((*item)->type()) {}
+
+  virtual inline void print (String *str, enum_query_type query_type)
+  {
+    if (ref)
+      (*ref)->print(str, query_type);
+    else
+      Item_ident::print(str, query_type);
+  }
+  virtual Ref_Type ref_type() { return AGGREGATE_REF; }
+};
+
+
 #ifdef MYSQL_SERVER
 #include "gstream.h"
 #include "spatial.h"

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2008-08-28 09:54:50 +0000
+++ b/sql/sql_select.cc	2008-10-02 10:25:19 +0000
@@ -14804,6 +14804,7 @@ setup_copy_fields(THD *thd, TMP_TABLE_PA
   Item *pos;
   List_iterator_fast<Item> li(all_fields);
   Copy_field *copy= NULL;
+  IF_DBUG(Copy_field *copy_start);
   res_selected_fields.empty();
   res_all_fields.empty();
   List_iterator_fast<Item> itr(res_all_fields);
@@ -14816,12 +14817,22 @@ setup_copy_fields(THD *thd, TMP_TABLE_PA
     goto err2;
 
   param->copy_funcs.empty();
+  IF_DBUG(copy_start= copy);
   for (i= 0; (pos= li++); i++)
   {
     Field *field;
     uchar *tmp;
     Item *real_pos= pos->real_item();
-    if (real_pos->type() == Item::FIELD_ITEM)
+    /* 
+      Process all fields that were not something else originally
+      and weren't substituted to Item_field later 
+    */
+    if (real_pos->type() == Item::FIELD_ITEM &&
+        !(pos->type() == Item::REF_ITEM &&
+          ((Item_ref *)pos)->ref_type() == Item_ref::AGGREGATE_REF &&
+          ((Item_aggregate_ref *)pos)->original_type != Item::FIELD_ITEM
+         )
+        )
     {
       Item_field *item;
       if (!(item= new Item_field(thd, ((Item_field*) real_pos))))
@@ -14868,6 +14879,7 @@ setup_copy_fields(THD *thd, TMP_TABLE_PA
 	  goto err;
         if (copy)
         {
+          DBUG_ASSERT (param->field_count > (uint) (copy - copy_start));
           copy->set(tmp, item->result_field);
           item->result_field->move_field(copy->to_ptr,copy->to_null_ptr,1);
 #ifdef HAVE_purify