← Back to team overview

maria-developers team mailing list archive

Please review MDEV-10124 Incorrect usage of CUBE/ROLLUP and ORDER BY with GROUP_CONCAT(a ORDER BY a)

 

Hi Sergei,

Please review MDEV-10124

Thanks.


Here's the story of the related code:


1. The original patch from Wax
commit: 0b505fb437eedd1b31c99888247c2259539c095b
date: Tue Mar 18 03:07:40 2003

opt_gorder_clause reused the regular order_clause,
which already had some protection against ROLLUP queries:

order_clause:
        ORDER_SYM BY
        {
          LEX *lex=Lex;
          if (lex->current_select->linkage != GLOBAL_OPTIONS_TYPE &&
              lex->current_select->select_lex()->olap !=
              UNSPECIFIED_OLAP_TYPE)
          {
            net_printf(lex->thd, ER_WRONG_USAGE,
                       "CUBE/ROLLUP",
                       "ORDER BY");
            YYABORT;
          }
        } order_list;

The assumption that ORDER BY in group_concat() had to have
the same ROLLUP restriction (with order_clause) was wrong.

Moreover, GROUP_CONCAT() in select_item_list was not affected
by this restriction, because WITH ROLLUP goes after
select_item_list and therefore sel->olap is always equal
to UNSPECIFIED_OLAP_TYPE during select_item_list.

GROUP BY was not affected:
- it goes before WITH ROLLUP and sel->olap is still
  UNSPECIFIED_OLAP_TYPE
- Aggregate functions like AVG(), GROUP_CONCAT() in GROUP BY
  are not allowed

So only GROUP_CONCAT() in HAVING and ORDER BY clauses
were erroneously affected by this restriction.


2. Bug#27848 rollup in union part causes error with order of union
commit: 3f6073ae63f9cb738cb6f0a6a8136e1d21ba0e1b
Author: unknown <igor@xxxxxxxxxxxxxx>  2007-12-15 01:42:46

The condition in the ROLLUP protection code became more complex.
Note, opt_gconcat_order still reuses the regular order_clause.


3. Bug#16347426 ASSERTION FAILED: (SELECT_INSERT &&
                 !TABLES->NEXT_NAME_RESOLUTION_TABLE) || !TAB
commit: 2d83663380f5c0ea720e31f51898912b0006cd9f
author: Chaithra Gopalareddy <chaithra.gopalareddy@xxxxxxxxxx>
date: 2013-04-14 06:00:49

opt_gorder_clause was refactored not to use order_clause and
collect information directly to select->gorder_list.
The ROLLUP protection code was duplicated from order_clause
to the new version of opt_gorder_clause.


diff --git a/mysql-test/r/func_gconcat.result b/mysql-test/r/func_gconcat.result
index 9014450..e5f9388 100644
--- a/mysql-test/r/func_gconcat.result
+++ b/mysql-test/r/func_gconcat.result
@@ -1200,3 +1200,51 @@ Warning	1260	Row 3 was cut by GROUP_CONCAT()
 Warning	1260	Row 5 was cut by GROUP_CONCAT()
 DROP TABLE t1;
 SET group_concat_max_len= DEFAULT;
+#
+# Start of 10.2 tests
+#
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (10),(20),(30);
+SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP;
+a	GROUP_CONCAT(a ORDER BY a)
+10	10
+20	20
+30	30
+NULL	10,20,30
+CREATE VIEW v1 AS
+SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP;
+SELECT * FROM v1;
+a	GROUP_CONCAT(a ORDER BY a)
+10	10
+20	20
+30	30
+NULL	10,20,30
+DROP VIEW v1;
+SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30';
+a	GROUP_CONCAT(a ORDER BY a)
+NULL	10,20,30
+CREATE VIEW v1 AS
+SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30';
+SELECT * FROM v1;
+a	GROUP_CONCAT(a ORDER BY a)
+NULL	10,20,30
+DROP VIEW v1;
+SELECT * FROM (SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30') t1;
+a	GROUP_CONCAT(a ORDER BY a)
+NULL	10,20,30
+CREATE VIEW v1 AS
+SELECT * FROM (SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30') t1;
+ERROR HY000: View's SELECT contains a subquery in the FROM clause
+SELECT (SELECT GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30');
+(SELECT GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30')
+10,20,30
+CREATE VIEW v1 AS
+SELECT (SELECT GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30');
+SELECT * FROM v1;
+Name_exp_1
+10,20,30
+DROP VIEW v1;
+DROP TABLE t1;
+#
+# End of 10.2 tests
+#
diff --git a/mysql-test/t/func_gconcat.test b/mysql-test/t/func_gconcat.test
index 3cc2443..3866046 100644
--- a/mysql-test/t/func_gconcat.test
+++ b/mysql-test/t/func_gconcat.test
@@ -873,3 +873,39 @@ SELECT LENGTH(GROUP_CONCAT(f1 ORDER BY f2)) FROM t1 GROUP BY f2;
 
 DROP TABLE t1;
 SET group_concat_max_len= DEFAULT;
+
+
+--echo #
+--echo # Start of 10.2 tests
+--echo #
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (10),(20),(30);
+
+SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP;
+CREATE VIEW v1 AS
+SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP;
+SELECT * FROM v1;
+DROP VIEW v1;
+
+SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30';
+CREATE VIEW v1 AS
+SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30';
+SELECT * FROM v1;
+DROP VIEW v1;
+
+SELECT * FROM (SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30') t1;
+--error ER_VIEW_SELECT_DERIVED
+CREATE VIEW v1 AS
+SELECT * FROM (SELECT a,GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30') t1;
+
+SELECT (SELECT GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30');
+CREATE VIEW v1 AS
+SELECT (SELECT GROUP_CONCAT(a ORDER BY a) FROM t1 GROUP BY a WITH ROLLUP HAVING GROUP_CONCAT(a ORDER BY a)='10,20,30');
+SELECT * FROM v1;
+
+DROP VIEW v1;
+DROP TABLE t1;
+
+--echo #
+--echo # End of 10.2 tests
+--echo #
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index 37a543a..78e85a3 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -10743,20 +10743,7 @@ opt_gconcat_separator:
 
 opt_gorder_clause:
           /* empty */
-        | ORDER_SYM BY
-          {
-            LEX *lex= Lex;
-            SELECT_LEX *sel= lex->current_select;
-            if (sel->linkage != GLOBAL_OPTIONS_TYPE &&
-                sel->olap != UNSPECIFIED_OLAP_TYPE &&
-                (sel->linkage != UNION_TYPE || sel->braces))
-            {
-              my_error(ER_WRONG_USAGE, MYF(0),
-                       "CUBE/ROLLUP", "ORDER BY");
-              MYSQL_YYABORT;
-            }
-          }
-         gorder_list;
+        | ORDER_SYM BY gorder_list;
         ;
 
 gorder_list:

Follow ups