← Back to team overview

maria-developers team mailing list archive

Re: fcf2c5fff14: MDEV-26299: Some views force server (and mysqldump) to generate invalid SQL for their definitions

 

Hi!

Sergei Golubchik <serg@xxxxxxxxxxx> 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@xxxxxxxxxxx
>

Follow ups

References