maria-developers team mailing list archive

[Rucha Deodhar <ruchad1998@xxxxxxxxx>]

On Fri, Jul 16, 2021 at 10:42 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Rucha!
>
> Hi, Sergei!
Thanks for the review.


> On Jul 15, Rucha Deodhar wrote:
> > revision-id: a11756e4fba (mariadb-10.5.11-27-ga11756e4fba)
> > parent(s): f0f47cbca18
> > author: Rucha Deodhar <rucha.deodhar@xxxxxxxxxxx>
> > committer: Rucha Deodhar <rucha.deodhar@xxxxxxxxxxx>
> > 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? ^^^
>

In both the cases above where the asterisk is not qualified by the table we
are inserting into
we get Unknown table error. Will add these to the test.


> > 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:
>
>
Will do.


> ============================
> --- 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 security@xxxxxxxxxxx
>
>
Regards,
Rucha Deodhar

> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp
>