maria-developers team mailing list archive

[Oleksandr Byelkin <sanja@xxxxxxxxxxxxxxxx>]

Hi!

On 16.03.2016 20:42, Sergei Golubchik wrote:
> 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);
OK!
> >         }
> >         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?
we merge view and so its ORDER BY first we resolve it in  view then in 
outer query. It actually works (I am not sure if I should include also 
following test:

create table t1 (a int, b int);
insert into t1 values (1,2), (2,1);
create view v1 as select a, b from t1 order by 1;
select b,a from v1;
b    a
2    1
1    2
select a,b from v1;
a    b
1    2
2    1
prepare stmt from "select b,a from v1";
execute stmt;
b    a
2    1
1    2
execute stmt;
b    a
2    1
1    2
prepare stmt from "select a,b from v1";
execute stmt;
a    b
1    2
2    1
execute stmt;
a    b
1    2
2    1
drop view v1;
drop table t1;


Actually I can make above even more efficient, just exit in case if 
resolving already done. Try it?
> > +      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?
For PS and exactly for PS  with "ORDER BY ?" to make new resolving for 
re-execution.
> >     if (join)
> >     {
> >       DBUG_ASSERT((st_select_lex*)join->select_lex == this);
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx
>
> _______________________________________________
> 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