[Maria-developers] 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@olga.mysql.com> 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@oracle.com> 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.
Hi, Alexander! On May 26, Alexander Barkov wrote:
Hi Sergei,
Please review MDEV-10124
Here's the story of the related code:
Thanks, that was very hepful!
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; ;
Ok to push Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (2)
-
Alexander Barkov
-
Sergei Golubchik