Backport into build-201002030816-5.0.87sp1

> ------------------------------------------------------------
> revno: 2818.4.1
> revision-id: alexey.kopytov@sun.com-20091030155453-0vlfwki805h9os62
> parent: joerg@mysql.com-20091016122941-rf6z0keqvmlgjfto
> committer: Alexey Kopytov <Alexey.Kopytov@Sun.com>
> branch nick: my50-bug48131
> timestamp: Fri 2009-10-30 18:54:53 +0300
> message:
>   Bug #48131: crash group by with rollup, distinct, filesort,
>               with temporary tables
>   
>   There were two problems the test case from this bug was
>   triggering:
>   
>   1. JOIN::rollup_init() was supposed to wrap all constant Items
>   into another object for queries with the WITH ROLLUP modifier
>   to ensure they are never considered as constants and therefore
>   are written into temporary tables if the optimizer chooses to
>   employ them for DISTINCT/GROUP BY handling.
>   
>   However, JOIN::rollup_init() was called before
>   make_join_statistics(), so Items corresponding to fields in
>   const tables could not be handled as intended, which was
>   causing all kinds of problems later in the query execution. In
>   particular, create_tmp_table() assumed all constant items
>   except "hidden" ones to be removed earlier by remove_const()
>   which led to improperly initialized Field objects for the
>   temporary table being created. This is what was causing crashes
>   and valgrind errors in storage engines.
>   
>   2. Even when the above problem had been fixed, the query from
>   the test case produced incorrect results due to some
>   DISTINCT/GROUP BY optimizations being performed by the
>   optimizer that are inapplicable in the WITH ROLLUP case.
>   
>   Fixed by disabling inapplicable DISTINCT/GROUP BY optimizations
>   when the WITH ROLLUP modifier is present, and splitting the
>   const-wrapping part of JOIN::rollup_init() into a separate
>   method which is now invoked after make_join_statistics() when
>   the const tables are already known.

mysql-test/r/olap.result

@@ -733,4 +733,24 @@ SELECT 1 FROM t1 GROUP BY (DATE(NULL)) WITH ROLLUP;
 1
 1
 DROP TABLE t1;
+#
+# Bug #48131: crash group by with rollup, distinct,
+#             filesort, with temporary tables
+#
+CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY);
+INSERT INTO t1 VALUES (1), (2);
+CREATE TABLE t2 (b INT);
+INSERT INTO t2 VALUES (100);
+SELECT a, b FROM t1, t2 GROUP BY a, b WITH ROLLUP;
+a	b
+1	100
+1	NULL
+2	100
+2	NULL
+NULL	NULL
+SELECT DISTINCT b FROM t1, t2 GROUP BY a, b WITH ROLLUP;
+b
+100
+NULL
+DROP TABLE t1, t2;
 End of 5.0 tests

mysql-test/t/olap.test

@@ -375,4 +375,19 @@ INSERT INTO t1 VALUES(0);
 SELECT 1 FROM t1 GROUP BY (DATE(NULL)) WITH ROLLUP;
 DROP TABLE t1;
 
+--echo #
+--echo # Bug #48131: crash group by with rollup, distinct,
+--echo #             filesort, with temporary tables
+--echo #
+
+CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY);
+INSERT INTO t1 VALUES (1), (2);
+CREATE TABLE t2 (b INT);
+INSERT INTO t2 VALUES (100);
+
+SELECT a, b FROM t1, t2 GROUP BY a, b WITH ROLLUP;
+SELECT DISTINCT b FROM t1, t2 GROUP BY a, b WITH ROLLUP;
+
+DROP TABLE t1, t2;
+
 --echo End of 5.0 tests

sql/sql_select.cc

@@ -928,6 +928,12 @@ JOIN::optimize()
     DBUG_RETURN(1);
   }
 
+  if (select_lex->olap == ROLLUP_TYPE && rollup_process_const_fields())
+  {
+    DBUG_PRINT("error", ("Error: rollup_process_fields() failed"));
+    DBUG_RETURN(1);
+  }
+
   /* Remove distinct if only const tables */
   select_distinct= select_distinct && (const_tables != tables);
   thd_proc_info(thd, "preparing");
@@ -1055,7 +1061,7 @@ JOIN::optimize()
        join_tab[const_tables].select->quick->get_type() != 
        QUICK_SELECT_I::QS_TYPE_GROUP_MIN_MAX))
   {
-    if (group_list &&
+    if (group_list && rollup.state == ROLLUP::STATE_NONE &&
        list_contains_unique_index(join_tab[const_tables].table,
                                  find_field_in_order_list,
                                  (void *) group_list))
@@ -1093,7 +1099,8 @@ JOIN::optimize()
     if (! hidden_group_fields && rollup.state == ROLLUP::STATE_NONE)
       select_distinct=0;
   }
-  else if (select_distinct && tables - const_tables == 1)
+  else if (select_distinct && tables - const_tables == 1 &&
+           rollup.state == ROLLUP::STATE_NONE)
   {
     /*
       We are only using one table. In this case we change DISTINCT to a
@@ -9870,6 +9877,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields,
     for (; cur_group ; cur_group= cur_group->next, key_part_info++)
     {
       Field *field=(*cur_group->item)->get_tmp_table_field();
+      DBUG_ASSERT(field->table == table);
       bool maybe_null=(*cur_group->item)->maybe_null;
       key_part_info->null_bit=0;
       key_part_info->field=  field;
@@ -15015,9 +15023,29 @@ bool JOIN::rollup_init()
       {
         item->maybe_null= 1;
         found_in_group= 1;
-        if (item->const_item())
+        break;
+      }
+    }
+    if (item->type() == Item::FUNC_ITEM && !found_in_group)
     {
+      bool changed= FALSE;
+      if (change_group_ref(thd, (Item_func *) item, group_list, &changed))
+        return 1;
       /*
+        We have to prevent creation of a field in a temporary table for
+        an expression that contains GROUP BY attributes.
+        Marking the expression item as 'with_sum_func' will ensure this.
+      */ 
+      if (changed)
+        item->with_sum_func= 1;
+    }
+  }
+  return 0;
+}
+
+/**
+   Wrap all constant Items in GROUP BY list.
+
    For ROLLUP queries each constant item referenced in GROUP BY list
    is wrapped up into an Item_func object yielding the same value
    as the constant item. The objects of the wrapper class are never
@@ -15029,7 +15057,27 @@ bool JOIN::rollup_init()
    ROLLUP is used together with DISTINCT in the SELECT list.
    Usually when creating temporary tables for a intermidiate
    result we do not include fields for constant expressions.
-	  */           
+
+   @retval
+     0  if ok
+   @retval
+     1  on error
+*/
+
+bool JOIN::rollup_process_const_fields()
+{
+  ORDER *group_tmp;
+  Item *item;
+  List_iterator<Item> it(all_fields);
+
+  for (group_tmp= group_list; group_tmp; group_tmp= group_tmp->next)
+  {
+    if (!(*group_tmp->item)->const_item())
+      continue;
+    while ((item= it++))
+    {
+      if (*group_tmp->item == item)
+      {
         Item* new_item= new Item_func_rollup_const(item);
         if (!new_item)
           return 1;
@@ -15040,22 +15088,10 @@ bool JOIN::rollup_init()
           if (*tmp->item == item)
             thd->change_item_tree(tmp->item, new_item);
         }
+        break;
       }
     }
-    }
-    if (item->type() == Item::FUNC_ITEM && !found_in_group)
-    {
-      bool changed= FALSE;
-      if (change_group_ref(thd, (Item_func *) item, group_list, &changed))
-        return 1;
-      /*
-        We have to prevent creation of a field in a temporary table for
-        an expression that contains GROUP BY attributes.
-        Marking the expression item as 'with_sum_func' will ensure this.
-      */ 
-      if (changed)
-        item->with_sum_func= 1;
-    }
+    it.rewind();
   }
   return 0;
 }

sql/sql_select.h

@@ -452,6 +452,7 @@ class JOIN :public Sql_alloc
   }
 
   bool rollup_init();
+  bool rollup_process_const_fields();
   bool rollup_make_fields(List<Item> &all_fields, List<Item> &fields,
 			  Item_sum ***func);
   int rollup_send_data(uint idx);