Fixed crashing bug in GROUP_CONCAT with ROLLUP

Fixed MDEV-4002: Server crash or valgrind errors in Item_func_group_concat::setup and Item_func_group_concat::add

mysql-test/r/group_by.result:
  Added test case for failing GROUP_CONCAT ... ROLLUP queries
mysql-test/t/group_by.test:
  Added test case for failing GROUP_CONCAT ... ROLLUP queries
sql/item_sum.cc:
  Fixed issue where field->table pointed to different temporary table than expected.
  Ensure that order->next points to the right object (could cause problems with setup_order())

3 files changed, 108 insertions, 9 deletions

diff --git a/mysql-test/r/group_by.result b/mysql-test/r/group_by.result
index 9f942747594..9455efbc0a6 100644
--- a/mysql-test/r/group_by.result
+++ b/mysql-test/r/group_by.result
@@ -2160,3 +2160,48 @@ f1	MIN(f2)	MAX(f2)
 4	00:25:00	00:25:00
 DROP TABLE t1;
 #End of test#49771
+#
+# Test of bug in GROUP_CONCAT with ROLLUP
+#
+CREATE TABLE t1 ( b VARCHAR(8) NOT NULL, a INT NOT NULL ) ENGINE=MyISAM;
+INSERT INTO t1 (a,b) VALUES (1,'c'),(2,'v');
+CREATE TABLE t2 ( c VARCHAR(8), d INT, KEY (c, d) ) ENGINE=MyISAM;
+INSERT INTO t2 VALUES ('v',6),('c',4),('v',3);
+SELECT b, GROUP_CONCAT( a, b ORDER BY a, b )
+FROM t1 JOIN t2 ON c = b GROUP BY b;
+b	GROUP_CONCAT( a, b ORDER BY a, b )
+c	1c
+v	2v,2v
+SELECT b, GROUP_CONCAT( a, b ORDER BY a, b )
+FROM t1 JOIN t2 ON c = b GROUP BY b WITH ROLLUP;
+b	GROUP_CONCAT( a, b ORDER BY a, b )
+c	1c
+v	2v,2v
+NULL	1c,2v,2v
+DROP TABLE t1,t2;
+#
+# Test of MDEV-4002
+#
+CREATE TABLE t1 ( 
+pk INT NOT NULL PRIMARY KEY, 
+d1 DOUBLE, 
+d2 DOUBLE, 
+i INT NOT NULL DEFAULT '0',
+KEY (i)
+) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1,1.0,1.1,1),(2,2.0,2.2,2);
+PREPARE stmt FROM " 
+SELECT DISTINCT i, GROUP_CONCAT(  d1, d2 ORDER BY d1, d2 ) 
+FROM t1 a1 NATURAL JOIN t1 a2 GROUP BY i WITH ROLLUP 
+";
+EXECUTE stmt;
+i	GROUP_CONCAT(  d1, d2 ORDER BY d1, d2 )
+1	11.1
+2	22.2
+NULL	11.1,22.2
+EXECUTE stmt;
+i	GROUP_CONCAT(  d1, d2 ORDER BY d1, d2 )
+1	11.1
+2	22.2
+NULL	11.1,22.2
+DROP TABLE t1;
diff --git a/mysql-test/t/group_by.test b/mysql-test/t/group_by.test
index 3af531418c5..4a7a4765385 100644
--- a/mysql-test/t/group_by.test
+++ b/mysql-test/t/group_by.test
@@ -1,3 +1,4 @@
+--source include/have_innodb.inc
 
 # Initialise
 --disable_warnings
@@ -1507,3 +1508,45 @@ SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1;
 
 DROP TABLE t1;
 --echo #End of test#49771
+
+--echo #
+--echo # Test of bug in GROUP_CONCAT with ROLLUP
+--echo #
+
+CREATE TABLE t1 ( b VARCHAR(8) NOT NULL, a INT NOT NULL ) ENGINE=MyISAM;
+INSERT INTO t1 (a,b) VALUES (1,'c'),(2,'v');
+
+CREATE TABLE t2 ( c VARCHAR(8), d INT, KEY (c, d) ) ENGINE=MyISAM;
+INSERT INTO t2 VALUES ('v',6),('c',4),('v',3);
+
+SELECT b, GROUP_CONCAT( a, b ORDER BY a, b )
+FROM t1 JOIN t2 ON c = b GROUP BY b;
+
+SELECT b, GROUP_CONCAT( a, b ORDER BY a, b )
+FROM t1 JOIN t2 ON c = b GROUP BY b WITH ROLLUP;
+
+DROP TABLE t1,t2;
+
+--echo #
+--echo # Test of MDEV-4002
+--echo #
+
+CREATE TABLE t1 ( 
+  pk INT NOT NULL PRIMARY KEY, 
+  d1 DOUBLE, 
+  d2 DOUBLE, 
+  i INT NOT NULL DEFAULT '0',
+  KEY (i)
+) ENGINE=InnoDB;
+
+INSERT INTO t1 VALUES (1,1.0,1.1,1),(2,2.0,2.2,2);
+
+PREPARE stmt FROM " 
+SELECT DISTINCT i, GROUP_CONCAT(  d1, d2 ORDER BY d1, d2 ) 
+FROM t1 a1 NATURAL JOIN t1 a2 GROUP BY i WITH ROLLUP 
+";
+
+EXECUTE stmt;
+EXECUTE stmt;
+
+DROP TABLE t1;
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index e00ef3e592c..58d605478af 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -2905,13 +2905,12 @@ int group_concat_key_cmp_with_distinct(void* arg, const void* key1,
                                        const void* key2)
 {
   Item_func_group_concat *item_func= (Item_func_group_concat*)arg;
-  TABLE *table= item_func->table;
 
   for (uint i= 0; i < item_func->arg_count_field; i++)
   {
     Item *item= item_func->args[i];
     /* 
-      If field_item is a const item then either get_tp_table_field returns 0
+      If field_item is a const item then either get_tmp_table_field returns 0
       or it is an item over a const table. 
     */
     if (item->const_item())
@@ -2923,7 +2922,8 @@ int group_concat_key_cmp_with_distinct(void* arg, const void* key1,
     */
     Field *field= item->get_tmp_table_field();
     int res;
-    uint offset= field->offset(field->table->record[0])-table->s->null_bytes;
+    uint offset= (field->offset(field->table->record[0]) -
+                  field->table->s->null_bytes);
     if((res= field->cmp((uchar*)key1 + offset, (uchar*)key2 + offset)))
       return res;
   }
@@ -2941,28 +2941,37 @@ int group_concat_key_cmp_with_order(void* arg, const void* key1,
 {
   Item_func_group_concat* grp_item= (Item_func_group_concat*) arg;
   ORDER **order_item, **end;
-  TABLE *table= grp_item->table;
 
   for (order_item= grp_item->order, end=order_item+ grp_item->arg_count_order;
        order_item < end;
        order_item++)
   {
     Item *item= *(*order_item)->item;
+    /* 
+      If field_item is a const item then either get_tmp_table_field returns 0
+      or it is an item over a const table. 
+    */
+    if (item->const_item())
+      continue;
     /*
       We have to use get_tmp_table_field() instead of
       real_item()->get_tmp_table_field() because we want the field in
       the temporary table, not the original field
+
+      Note that for the case of ROLLUP, field may point to another table
+      tham grp_item->table. This is howver ok as the table definitions are
+      the same.
     */
     Field *field= item->get_tmp_table_field();
     /* 
-      If item is a const item then either get_tp_table_field returns 0
+      If item is a const item then either get_tmp_table_field returns 0
       or it is an item over a const table. 
     */
-    if (field && !item->const_item())
+    if (field)
     {
       int res;
       uint offset= (field->offset(field->table->record[0]) -
-                    table->s->null_bytes);
+                    field->table->s->null_bytes);
       if ((res= field->cmp((uchar*)key1 + offset, (uchar*)key2 + offset)))
         return (*order_item)->asc ? res : -res;
     }
@@ -3156,12 +3165,13 @@ Item_func_group_concat::Item_func_group_concat(THD *thd,
   */
   ORDER *tmp;
   if (!(tmp= (ORDER *) thd->alloc(sizeof(ORDER *) * arg_count_order +
-                                     sizeof(ORDER) * arg_count_order)))
+                                  sizeof(ORDER) * arg_count_order)))
     return;
   order= (ORDER **)(tmp + arg_count_order);
   for (uint i= 0; i < arg_count_order; i++, tmp++)
   {
     memcpy(tmp, item->order[i], sizeof(ORDER));
+    tmp->next= i == arg_count_order-1 ? 0 : tmp+1;
     order[i]= tmp;
   }
 }
@@ -3445,7 +3455,8 @@ bool Item_func_group_concat::setup(THD *thd)
   */
   if (!(table= create_tmp_table(thd, tmp_table_param, all_fields,
                                 (ORDER*) 0, 0, TRUE,
-                                (select_lex->options | thd->variables.option_bits),
+                                (select_lex->options |
+                                 thd->variables.option_bits),
                                 HA_POS_ERROR, (char*) "")))
     DBUG_RETURN(TRUE);
   table->file->extra(HA_EXTRA_NO_ROWS);