Hi, Rucha! On Oct 23, Rucha Deodhar wrote: > revision-id: 6d881a271a9 (mariadb-10.5.4-108-g6d881a271a9) > parent(s): bbd70fcc43c > author: Rucha Deodhar <rucha.deod...@mariadb.com> > committer: Rucha Deodhar <rucha.deod...@mariadb.com> > timestamp: 2020-10-23 12:57:02 +0530 > message: > > MDEV-23178: Qualified asterisk not supported in INSERT .. RETURNING > > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 45ce4be3eb5..4ce7b034029 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -7959,7 +7959,7 @@ insert_fields(THD *thd, Name_resolution_context > *context, const char *db_name, > bool any_privileges, uint *hidden_bit_fields) > { > Field_iterator_table_ref field_iterator; > - bool found; > + bool found, is_insert_or_replace_returning= false; > char name_buff[SAFE_NAME_LEN+1]; > DBUG_ENTER("insert_fields"); > DBUG_PRINT("arena", ("stmt arena: %p",thd->stmt_arena)); > @@ -7978,13 +7978,31 @@ insert_fields(THD *thd, Name_resolution_context > *context, const char *db_name, > > found= FALSE; > > + if ((thd->lex->sql_command == SQLCOM_INSERT_SELECT || > + thd->lex->sql_command == SQLCOM_REPLACE_SELECT || > + thd->lex->sql_command == SQLCOM_INSERT || > + thd->lex->sql_command == SQLCOM_REPLACE) && > + thd->lex->has_returning()) > + is_insert_or_replace_returning= true; > + > /* > If table names are qualified, then loop over all tables used in the > query, > else treat natural joins as leaves and do not iterate over their > underlying > tables. > + Also, When we have INSERT/REPLACE...RETURNING and have qualified > asterisk, > + table_name is not NULL and context->table_list is either NULL or has > + incorrect reference because context->table_list has tables from the > + FROM clause. > + context->table_list has incorrect reference (has table from the FROM > clause > + instead of table we are inserting into) for > + INSERT/REPLACE...SELECT...RETURNING because we have a FROM clause from > the > + SELECT statement. For INSERT/REPLACE...RETURNING it is NULL because > + there is no FROM clause. > */ > - for (TABLE_LIST *tables= (table_name ? context->table_list : > - context->first_name_resolution_table); > + for (TABLE_LIST *tables= (table_name ? (is_insert_or_replace_returning ? > + > context->first_name_resolution_table : > + context->table_list) : > + (context->first_name_resolution_table));
ok, I've compiled it to run few queries, now I see that this is wrong. you need to understand the old code and the old comment before modifying them. one way to do it is to change the condition from table_name ? context->table_list : context->first_name_resolution_table to simply context->first_name_resolution_table and see what breaks. You'll see that main.type_varchar will break with At line 319: query 'SELECT t1.* FROM t1 LEFT JOIN t2 USING (c1)' failed: 1051: Unknown table 'test.t1' indeed, t1 is only present in context->table_list, but not in context->first_name_resolution_table. So after your fix the following statement will no longer work: INSERT INTO t2 SELECT t1.* FROM t1 JOIN t1 AS t USING (id1) WHERE id1=1 RETURNING *; it's because you set is_insert_or_replace_returning both for the SELECT and for the RETURNING clauses. > tables; > tables= (table_name ? tables->next_local : > tables->next_name_resolution_table) also this looks suspicious. Normally one starts from table_list and follows the next_local pointer or one starts from first_name_resolution_table and follows the next_name_resolution_table pointer. Your change creates a strange mix of both, it looks like it would be wrong. Regards, Sergei VP of MariaDB Server Engineering and secur...@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