Hi, Rucha!
On Jul 15, Rucha Deodhar wrote:
> revision-id: a11756e4fba (mariadb-10.5.11-27-ga11756e4fba)
> parent(s): f0f47cbca18
> author: Rucha Deodhar <[email protected]>
> committer: Rucha Deodhar <[email protected]>
> timestamp: 2021-07-05 20:14:19 +0530
> message:
>
> MDEV-23178: Qualified asterisk not supported in INSERT .. RETURNING
>
> Analysis: When we have INSERT/REPLACE returning with qualified asterisk in the
> RETURNING clause, '*' is not resolved properly because of wrong context.
> context->table_list is NULL or has incorrect table because context->table_list
> has tables from the FROM clause.
> Fix: If filling fields instead of '*' for qualified asterisk in RETURNING,
> use first_name_resolution_table for correct resolution of item.
please, add a couple of tests with a qualified asterisk, where it's
qualified _not_ by the table you're inserting into. Like
INSERT INTO t1(id1,val1) VALUES(14,'m') RETURNING t2.*
not very interesting case, an obvious error ^^^
INSERT INTO t2 SELECT t1.* FROM t1 WHERE id1=2 RETURNING t1.*;
but what will happen here? ^^^
> diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> index 1476d708d75..60d14c6dbea 100644
> --- a/sql/sql_base.cc
> +++ b/sql/sql_base.cc
> @@ -8032,12 +8032,13 @@ insert_fields(THD *thd, Name_resolution_context
> *context, const char *db_name,
> else treat natural joins as leaves and do not iterate over their
> underlying
> tables.
> */
> - for (TABLE_LIST *tables= (table_name ? context->table_list :
> - context->first_name_resolution_table);
> + TABLE_LIST *tables= returning_field || !table_name ?
> context->first_name_resolution_table :
> + context->table_list;
> + for (;
> tables;
> - tables= (table_name ? tables->next_local :
> - tables->next_name_resolution_table)
> + tables= returning_field || !table_name ?
> tables->next_name_resolution_table :
> + tables->next_local
> )
> {
> Field *field;
> TABLE *table= tables->table;
1. please also update the comment to mention RETURNING.
2. Could you apply the following patch as a separate commit before your
fix:
============================
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -8098,12 +8098,14 @@ insert_fields(THD *thd, Name_resolution_context
*context, const char *db_name,
else treat natural joins as leaves and do not iterate over their underlying
tables.
*/
- for (TABLE_LIST *tables= (table_name ? context->table_list :
- context->first_name_resolution_table);
- tables;
- tables= (table_name ? tables->next_local :
- tables->next_name_resolution_table)
- )
+ TABLE_LIST *first= context->first_name_resolution_table;
+ TABLE_LIST *TABLE_LIST::* next= &TABLE_LIST::next_name_resolution_table;
+ if (table_name)
+ {
+ first= context->table_list;
+ next= &TABLE_LIST::next_local;
+ }
+ for (TABLE_LIST *tables= first; tables; tables= tables->*next)
{
Field *field;
TABLE *table= tables->table;
============================
it'll remove the condition from inside the loop, will put it in one
place only. And will make your change simpler and clearer.
Regards,
Sergei
VP of MariaDB Server Engineering
and [email protected]
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
