Re: [Maria-developers] fcf2c5fff14: MDEV-26299: Some views force server (and mysqldump) to generate invalid SQL for their definitions
Hi, Oleksandr! On Oct 16, Oleksandr Byelkin wrote:
revision-id: fcf2c5fff14 (mariadb-10.2.40-78-gfcf2c5fff14) parent(s): d5a15f04f4a author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2021-10-01 14:46:22 +0200 message:
MDEV-26299: Some views force server (and mysqldump) to generate invalid SQL for their definitions
Do not print illegal table field names for non-top-level SELECT list, they will not be refered in any case but create problem for parsing of printed result.
diff --git a/sql/sql_select.cc b/sql/sql_select.cc index ff584e936b7..ebee9c85791 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -25797,6 +25797,11 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type)
//Item List bool first= 1; + /* + outer_select() can not be used here because it is for name resolution + and will return NULL at any end of name resolution chain (view/derived) + */ + bool top_level= (get_master()->get_master() == 0); List_iterator_fast<Item> it(item_list); Item *item; while ((item= it++)) @@ -25806,7 +25811,8 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type) else str->append(',');
- if (is_subquery_function() && item->is_autogenerated_name) + if ((is_subquery_function() && item->is_autogenerated_name) || + !item->name)
Where can item->name be NULL? I thought the either specified by the user or autogenerated, but never NULL.
{ /* Do not print auto-generated aliases in subqueries. It has no purpose @@ -25815,7 +25821,26 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type) item->print(str, query_type); } else - item->print_item_w_name(str, query_type); + { + /* + Despite presence of item->name_length it is incorrect in many cases + till we start using LEX_STRING for the name. + */ + size_t len= strlen(item->name); + + /* + Do not print illegal names (if it is not top level SELECT). + Top level view checked (and correct name are assigned), + other cases of top level SELECT are not important, because + it is not "table field". + */ + if (top_level || + !item->is_autogenerated_name || + !check_table_name(item->name, len, TRUE))
why check_table_name() if it isn't a table? it's an incorrect check to use for columns.
+ item->print_item_w_name(str, query_type); + else + item->print(str, query_type); + } }
/*
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi! Sergei Golubchik <serg@mariadb.org> schrieb am Sa., 16. Okt. 2021, 10:23:
Hi, Oleksandr!
On Oct 16, Oleksandr Byelkin wrote:
revision-id: fcf2c5fff14 (mariadb-10.2.40-78-gfcf2c5fff14) parent(s): d5a15f04f4a author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2021-10-01 14:46:22 +0200 message:
MDEV-26299: Some views force server (and mysqldump) to generate invalid SQL for their definitions
Do not print illegal table field names for non-top-level SELECT list, they will not be refered in any case but create problem for parsing of printed result.
diff --git a/sql/sql_select.cc b/sql/sql_select.cc index ff584e936b7..ebee9c85791 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -25797,6 +25797,11 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type)
//Item List bool first= 1; + /* + outer_select() can not be used here because it is for name resolution + and will return NULL at any end of name resolution chain (view/derived) + */ + bool top_level= (get_master()->get_master() == 0); List_iterator_fast<Item> it(item_list); Item *item; while ((item= it++)) @@ -25806,7 +25811,8 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type) else str->append(',');
- if (is_subquery_function() && item->is_autogenerated_name) + if ((is_subquery_function() && item->is_autogenerated_name) || + !item->name)
Where can item->name be NULL? I thought the either specified by the user or autogenerated, but never NULL.
Or internally created with null name.
{ /* Do not print auto-generated aliases in subqueries. It has no
purpose
@@ -25815,7 +25821,26 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type) item->print(str, query_type); } else - item->print_item_w_name(str, query_type); + { + /* + Despite presence of item->name_length it is incorrect in many cases + till we start using LEX_STRING for the name. + */ + size_t len= strlen(item->name); + + /* + Do not print illegal names (if it is not top level SELECT). + Top level view checked (and correct name are assigned), + other cases of top level SELECT are not important, because + it is not "table field". + */ + if (top_level || + !item->is_autogenerated_name || + !check_table_name(item->name, len, TRUE))
why check_table_name() if it isn't a table? it's an incorrect check to use for columns.
A derived table AKA a subquery in the FROM clause is still a table. For other things I have no idea why we check AS part for correspondence to table name rules, but we do it on parsing.
+ item->print_item_w_name(str, query_type); + else + item->print(str, query_type); + } }
/*
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi, Oleksandr!
@@ -25806,7 +25811,8 @@ void st_select_lex::print(THD *thd, String *str, else str->append(',');
- if (is_subquery_function() && item->is_autogenerated_name) + if ((is_subquery_function() && item->is_autogenerated_name) || + !item->name)
Where can item->name be NULL? I thought the either specified by the user or autogenerated, but never NULL.
Or internally created with null name.
Do you have an example of such internally created with null name item?
@@ -25815,7 +25821,26 @@ void st_select_lex::print(THD *thd, String + if (top_level || + !item->is_autogenerated_name || + !check_table_name(item->name, len, TRUE))
why check_table_name() if it isn't a table? it's an incorrect check to use for columns.
A derived table AKA a subquery in the FROM clause is still a table. For other things I have no idea why we check AS part for correspondence to table name rules, but we do it on parsing.
A derived table is still a table. But item->name isn't a name of a defived, it's a name of the item. check_table_name is just incorrect there, it will reject valid column names and will accept invalid column names. A check_column_name function exists for a reason :) Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (2)
-
Oleksandr Byelkin -
Sergei Golubchik