← Back to team overview

maria-developers team mailing list archive

Re: 72b709a: MDEV-9701: CREATE VIEW with GROUP BY or ORDER BY and constant produces invalid definition

 

Hi, Sanja!

On Mar 16, OleksandrByelkin wrote:
> revision-id: 72b709ac7503ae6dd2b5e9049322fefb90b0ebbe (mariadb-10.1.12-16-g72b709a)
> parent(s): 9b53d84d14a9b031d193f6beae382a232aa738e3
> committer: Oleksandr Byelkin
> timestamp: 2016-03-16 19:49:17 +0100
> message:
> 
> MDEV-9701: CREATE VIEW with GROUP BY or ORDER BY and constant produces invalid definition
> 
> Fixed printing integer constant in the ORDER clause (MySQL solution)
> Removed workaround for double resolving counter in the ORDER.
> 
> ---
>  mysql-test/r/view.result | 19 +++++++++++++++++++
>  mysql-test/t/view.test   | 17 +++++++++++++++++
>  sql/sql_lex.cc           | 32 ++++++++++++--------------------
>  sql/sql_select.cc        |  6 +++++-
>  sql/sql_union.cc         | 11 +++++++++++
>  5 files changed, 64 insertions(+), 21 deletions(-)
> 
> diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result
> index 8ee72c3..d4a2a9b 100644
> diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
> index fd84109..31fc5f9 100644
> --- a/sql/sql_lex.cc
> +++ b/sql/sql_lex.cc
> @@ -2689,30 +2689,22 @@ void st_select_lex::print_order(String *str,
>    {
>      if (order->counter_used)
>      {
> -      if (!(query_type & QT_VIEW_INTERNAL))
> -      {
>          char buffer[20];
>          size_t length= my_snprintf(buffer, 20, "%d", order->counter);
>          str->append(buffer, (uint) length);

you can replace these three lines with

   str->append_ulonglong(order->counter);

>        }
>        else
>        {
> -      /* replace numeric reference with expression */
> +      /* replace numeric reference with equivalent for ORDER constant */
>          if (order->item[0]->type() == Item::INT_ITEM &&
>              order->item[0]->basic_const_item())
>          {
> -          char buffer[20];
> -          size_t length= my_snprintf(buffer, 20, "%d", order->counter);
> -          str->append(buffer, (uint) length);
>            /* make it expression instead of integer constant */
> -        str->append(STRING_WITH_LEN("+0"));
> +        str->append(STRING_WITH_LEN("''"));
>          }
>          else
>            (*order->item)->print(str, query_type);
>        }
> -    }
> -    else
> -      (*order->item)->print(str, query_type);
>      if (!order->asc)
>        str->append(STRING_WITH_LEN(" desc"));
>      if (order->next)
> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index fff24b9..f14e8b1 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -21874,7 +21874,11 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
>    */
>    if (order_item->type() == Item::INT_ITEM && order_item->basic_const_item())
>    {						/* Order by position */
> -    uint count= (uint) order_item->val_int();
> +    uint count;
> +    if (order->counter_used)

how can this happen?

> +      count= order->counter; // counter was once resolved
> +    else
> +      count= (uint) order_item->val_int();
>      if (!count || count > fields.elements)
>      {
>        my_error(ER_BAD_FIELD_ERROR, MYF(0),
> diff --git a/sql/sql_union.cc b/sql/sql_union.cc
> index c835083..8145cec 100644
> --- a/sql/sql_union.cc
> +++ b/sql/sql_union.cc
> @@ -1161,11 +1161,22 @@ List<Item> *st_select_lex_unit::get_unit_column_types()
>    return &sl->item_list;
>  }
>  
> +
> +static void cleanup_order(ORDER *order)
> +{
> +  for (; order; order= order->next)
> +    order->counter_used= 0;
> +}
> +
> +
>  bool st_select_lex::cleanup()
>  {
>    bool error= FALSE;
>    DBUG_ENTER("st_select_lex::cleanup()");
>  
> +  cleanup_order(order_list.first);
> +  cleanup_order(group_list.first);

why is it needed?

>    if (join)
>    {
>      DBUG_ASSERT((st_select_lex*)join->select_lex == this);

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx


Follow ups