maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Rucha!

On Jul 22, Rucha Deodhar wrote:
> revision-id: d91c4eea668 (mariadb-10.5.11-28-gd91c4eea668)
> parent(s): b1e57179242
> author: Rucha Deodhar
> committer: Rucha Deodhar
> timestamp: 2021-07-17 18:47:15 +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. For INSERT/REPLACE...SELECT...RETURNING,
> context->table_list has table we are inserting from. While in other
> INSERT/REPLACE syntax, context->table_list is NULL because there is no FROM
> clause.
> Fix: If filling fields instead of '*' for qualified asterisk in RETURNING,
> use first_name_resolution_table for correct resolution of item.

> @@ -271,6 +289,8 @@ INSERT INTO t2(id2, val2) SELECT * FROM t1 WHERE id1=2 RETURNING(SELECT
>  --error ER_SUBQUERY_NO_1_ROW
>  INSERT INTO t2(id2,val2) SELECT * FROM t1 WHERE id1=2 RETURNING (SELECT
>  id2 FROM t2);
> +#--error ER_BAD_TABLE_ERROR
> +#INSERT INTO t2(id2,val2) SELECT t1.* FROM t1 WHERE id1=2 RETURNING t1.*;

Why is this test commented out?
(same for REPLACE)

> @@ -8031,10 +8034,20 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name,
>      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.
> +    If filling fields instead of '*' for qualified asterisk in RETURNING clause,
> +    use first_name_resolution_table for correct resolution of item because
> +    context->table_list is either NULL or has incorrect table because
> +    context->table_list has tables from the FROM clause.
> +    In case of INSERT/REPLACE...SELECT..RETURNING context->table_list will have
> +    table we are inserting and NULL for other cases because there is no
> +    FROM clause.
>    */
> -  TABLE_LIST *first= context->first_name_resolution_table;
> -  TABLE_LIST *TABLE_LIST::* next= &TABLE_LIST::next_name_resolution_table;
> -  if (table_name)
> +  if (returning_field || !table_name)
> +  {
> +    first= context->first_name_resolution_table;
> +    next= &TABLE_LIST::next_name_resolution_table;

These are the values already set before, so assignments are redundant.
Either remove initializers, like

     TABLE_LIST *first;
     TABLE_LIST *TABLE_LIST::* next;
     if (returning_field || !table_name)

or reverse your condition:

     TABLE_LIST *first= context->first_name_resolution_table;
     TABLE_LIST *TABLE_LIST::* next= &TABLE_LIST::next_name_resolution_table;
     if (table_name && !returning_field)

> +  }
> +  else
>    {
>      first= context->table_list;
>      next= &TABLE_LIST::next_local;

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx