Re: [Maria-developers] 72b709a: MDEV-9701: CREATE VIEW with GROUP BY or ORDER BY and constant produces invalid definition
Hi, Sanja! On Mar 16, OleksandrByelkin wrote:
revision-id: 72b709ac7503ae6dd2b5e9049322fefb90b0ebbe (mariadb-10.1.12-16-g72b709a) parent(s): 9b53d84d14a9b031d193f6beae382a232aa738e3 committer: Oleksandr Byelkin timestamp: 2016-03-16 19:49:17 +0100 message:
MDEV-9701: CREATE VIEW with GROUP BY or ORDER BY and constant produces invalid definition
Fixed printing integer constant in the ORDER clause (MySQL solution) Removed workaround for double resolving counter in the ORDER.
--- mysql-test/r/view.result | 19 +++++++++++++++++++ mysql-test/t/view.test | 17 +++++++++++++++++ sql/sql_lex.cc | 32 ++++++++++++-------------------- sql/sql_select.cc | 6 +++++- sql/sql_union.cc | 11 +++++++++++ 5 files changed, 64 insertions(+), 21 deletions(-)
diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 8ee72c3..d4a2a9b 100644 diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index fd84109..31fc5f9 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -2689,30 +2689,22 @@ void st_select_lex::print_order(String *str, { if (order->counter_used) { - if (!(query_type & QT_VIEW_INTERNAL)) - { char buffer[20]; size_t length= my_snprintf(buffer, 20, "%d", order->counter); str->append(buffer, (uint) length);
you can replace these three lines with str->append_ulonglong(order->counter);
} else { - /* replace numeric reference with expression */ + /* replace numeric reference with equivalent for ORDER constant */ if (order->item[0]->type() == Item::INT_ITEM && order->item[0]->basic_const_item()) { - char buffer[20]; - size_t length= my_snprintf(buffer, 20, "%d", order->counter); - str->append(buffer, (uint) length); /* make it expression instead of integer constant */ - str->append(STRING_WITH_LEN("+0")); + str->append(STRING_WITH_LEN("''")); } else (*order->item)->print(str, query_type); } - } - else - (*order->item)->print(str, query_type); if (!order->asc) str->append(STRING_WITH_LEN(" desc")); if (order->next) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index fff24b9..f14e8b1 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -21874,7 +21874,11 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, */ if (order_item->type() == Item::INT_ITEM && order_item->basic_const_item()) { /* Order by position */ - uint count= (uint) order_item->val_int(); + uint count; + if (order->counter_used)
how can this happen?
+ count= order->counter; // counter was once resolved + else + count= (uint) order_item->val_int(); if (!count || count > fields.elements) { my_error(ER_BAD_FIELD_ERROR, MYF(0), diff --git a/sql/sql_union.cc b/sql/sql_union.cc index c835083..8145cec 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -1161,11 +1161,22 @@ List<Item> *st_select_lex_unit::get_unit_column_types() return &sl->item_list; }
+ +static void cleanup_order(ORDER *order) +{ + for (; order; order= order->next) + order->counter_used= 0; +} + + bool st_select_lex::cleanup() { bool error= FALSE; DBUG_ENTER("st_select_lex::cleanup()");
+ cleanup_order(order_list.first); + cleanup_order(group_list.first);
why is it needed?
if (join) { DBUG_ASSERT((st_select_lex*)join->select_lex == this);
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi! On 16.03.2016 20:42, Sergei Golubchik wrote:
Hi, Sanja!
On Mar 16, OleksandrByelkin wrote:
revision-id: 72b709ac7503ae6dd2b5e9049322fefb90b0ebbe (mariadb-10.1.12-16-g72b709a) parent(s): 9b53d84d14a9b031d193f6beae382a232aa738e3 committer: Oleksandr Byelkin timestamp: 2016-03-16 19:49:17 +0100 message:
MDEV-9701: CREATE VIEW with GROUP BY or ORDER BY and constant produces invalid definition
Fixed printing integer constant in the ORDER clause (MySQL solution) Removed workaround for double resolving counter in the ORDER.
--- mysql-test/r/view.result | 19 +++++++++++++++++++ mysql-test/t/view.test | 17 +++++++++++++++++ sql/sql_lex.cc | 32 ++++++++++++-------------------- sql/sql_select.cc | 6 +++++- sql/sql_union.cc | 11 +++++++++++ 5 files changed, 64 insertions(+), 21 deletions(-)
diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 8ee72c3..d4a2a9b 100644 diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index fd84109..31fc5f9 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -2689,30 +2689,22 @@ void st_select_lex::print_order(String *str, { if (order->counter_used) { - if (!(query_type & QT_VIEW_INTERNAL)) - { char buffer[20]; size_t length= my_snprintf(buffer, 20, "%d", order->counter); str->append(buffer, (uint) length); you can replace these three lines with
str->append_ulonglong(order->counter); OK!
} else { - /* replace numeric reference with expression */ + /* replace numeric reference with equivalent for ORDER constant */ if (order->item[0]->type() == Item::INT_ITEM && order->item[0]->basic_const_item()) { - char buffer[20]; - size_t length= my_snprintf(buffer, 20, "%d", order->counter); - str->append(buffer, (uint) length); /* make it expression instead of integer constant */ - str->append(STRING_WITH_LEN("+0")); + str->append(STRING_WITH_LEN("''")); } else (*order->item)->print(str, query_type); } - } - else - (*order->item)->print(str, query_type); if (!order->asc) str->append(STRING_WITH_LEN(" desc")); if (order->next) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index fff24b9..f14e8b1 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -21874,7 +21874,11 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, */ if (order_item->type() == Item::INT_ITEM && order_item->basic_const_item()) { /* Order by position */ - uint count= (uint) order_item->val_int(); + uint count; + if (order->counter_used)
how can this happen? we merge view and so its ORDER BY first we resolve it in view then in outer query. It actually works (I am not sure if I should include also following test:
create table t1 (a int, b int); insert into t1 values (1,2), (2,1); create view v1 as select a, b from t1 order by 1; select b,a from v1; b a 2 1 1 2 select a,b from v1; a b 1 2 2 1 prepare stmt from "select b,a from v1"; execute stmt; b a 2 1 1 2 execute stmt; b a 2 1 1 2 prepare stmt from "select a,b from v1"; execute stmt; a b 1 2 2 1 execute stmt; a b 1 2 2 1 drop view v1; drop table t1; Actually I can make above even more efficient, just exit in case if resolving already done. Try it?
+ count= order->counter; // counter was once resolved + else + count= (uint) order_item->val_int(); if (!count || count > fields.elements) { my_error(ER_BAD_FIELD_ERROR, MYF(0), diff --git a/sql/sql_union.cc b/sql/sql_union.cc index c835083..8145cec 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -1161,11 +1161,22 @@ List<Item> *st_select_lex_unit::get_unit_column_types() return &sl->item_list; }
+ +static void cleanup_order(ORDER *order) +{ + for (; order; order= order->next) + order->counter_used= 0; +} + + bool st_select_lex::cleanup() { bool error= FALSE; DBUG_ENTER("st_select_lex::cleanup()");
+ cleanup_order(order_list.first); + cleanup_order(group_list.first); why is it needed?
For PS and exactly for PS with "ORDER BY ?" to make new resolving for re-execution.
if (join) { DBUG_ASSERT((st_select_lex*)join->select_lex == this);
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Hi, Oleksandr! On Mar 16, Oleksandr Byelkin wrote:
On Mar 16, OleksandrByelkin wrote:
revision-id: 72b709ac7503ae6dd2b5e9049322fefb90b0ebbe (mariadb-10.1.12-16-g72b709a) parent(s): 9b53d84d14a9b031d193f6beae382a232aa738e3 committer: Oleksandr Byelkin timestamp: 2016-03-16 19:49:17 +0100 message:
MDEV-9701: CREATE VIEW with GROUP BY or ORDER BY and constant produces invalid definition
Fixed printing integer constant in the ORDER clause (MySQL solution) Removed workaround for double resolving counter in the ORDER.
diff --git a/sql/sql_select.cc b/sql/sql_select.cc index fff24b9..f14e8b1 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -21874,7 +21874,11 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, */ if (order_item->type() == Item::INT_ITEM && order_item->basic_const_item()) { /* Order by position */ - uint count= (uint) order_item->val_int(); + uint count; + if (order->counter_used) how can this happen? we merge view and so its ORDER BY first we resolve it in view then in outer query. It actually works (I am not sure if I should include also following test:
create table t1 (a int, b int); insert into t1 values (1,2), (2,1); create view v1 as select a, b from t1 order by 1; select b,a from v1; b a 2 1 1 2
Did it work before this your change? If yes - how?
Actually I can make above even more efficient, just exit in case if resolving already done. Try it?
If you do - do it in a separate commit, please
+ count= order->counter; // counter was once resolved + else + count= (uint) order_item->val_int(); if (!count || count > fields.elements) { my_error(ER_BAD_FIELD_ERROR, MYF(0), diff --git a/sql/sql_union.cc b/sql/sql_union.cc index c835083..8145cec 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc bool st_select_lex::cleanup() { bool error= FALSE; DBUG_ENTER("st_select_lex::cleanup()");
+ cleanup_order(order_list.first); + cleanup_order(group_list.first); why is it needed? For PS and exactly for PS with "ORDER BY ?" to make new resolving for re-execution.
Why would one need to redo it? The number will always resolve to the same Item, won't it? One can safely do it only once, it seems. Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi, Oleksandr!
On Mar 16, Oleksandr Byelkin wrote:
On Mar 16, OleksandrByelkin wrote:
revision-id: 72b709ac7503ae6dd2b5e9049322fefb90b0ebbe (mariadb-10.1.12-16-g72b709a) parent(s): 9b53d84d14a9b031d193f6beae382a232aa738e3 committer: Oleksandr Byelkin timestamp: 2016-03-16 19:49:17 +0100 message:
MDEV-9701: CREATE VIEW with GROUP BY or ORDER BY and constant produces invalid definition
Fixed printing integer constant in the ORDER clause (MySQL solution) Removed workaround for double resolving counter in the ORDER.
diff --git a/sql/sql_select.cc b/sql/sql_select.cc index fff24b9..f14e8b1 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -21874,7 +21874,11 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, */ if (order_item->type() == Item::INT_ITEM && order_item->basic_const_item()) { /* Order by position */ - uint count= (uint) order_item->val_int(); + uint count; + if (order->counter_used) how can this happen? we merge view and so its ORDER BY first we resolve it in view then in outer query. It actually works (I am not sure if I should include also following test:
create table t1 (a int, b int); insert into t1 values (1,2), (2,1); create view v1 as select a, b from t1 order by 1; select b,a from v1; b a 2 1 1 2 Did it work before this your change? If yes - how? It worked because of IF which avoid printing order number in case of
On 17.03.2016 10:21, Sergei Golubchik wrote: printing for VIEW (expression was printed) so not 1 but a (or even constant in the case of the bug)
Actually I can make above even more efficient, just exit in case if resolving already done. Try it? If you do - do it in a separate commit, please
OK
+ count= order->counter; // counter was once resolved + else + count= (uint) order_item->val_int(); if (!count || count > fields.elements) { my_error(ER_BAD_FIELD_ERROR, MYF(0), diff --git a/sql/sql_union.cc b/sql/sql_union.cc index c835083..8145cec 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc bool st_select_lex::cleanup() { bool error= FALSE; DBUG_ENTER("st_select_lex::cleanup()");
+ cleanup_order(order_list.first); + cleanup_order(group_list.first); why is it needed? For PS and exactly for PS with "ORDER BY ?" to make new resolving for re-execution. Why would one need to redo it? The number will always resolve to the same Item, won't it? One can safely do it only once, it seems.
According to the test case we have in case of integer parameter 'ORDER BY ?' interpret as "by which column we will order", so result order differs depends on execution parameter.
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (2)
-
Oleksandr Byelkin -
Sergei Golubchik