← 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, 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.

>      {
>        /*
>          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.

> +        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