maria-developers team mailing list archive

[varun gupta <varunraiko1803@xxxxxxxxx>]

Hi Sergey,


On Thu, Aug 6, 2020 at 4:31 PM Sergey Petrunia <sergey@xxxxxxxxxxx> wrote:

> Hi Varun,
>
> First: I can still get the assertion failure if I run this:
>
> set @@sql_mode='only_full_group_by';
> select min(1 mod a1), bit_or(a2) over () from t1;
>
> As far as I understand, the query is not valid for
> only_full_group_by mode. (Window functions as such are ok,
> but here window function is using a2, which cannot be accessed in
> the post-grouping context).
>
> Well this is a good catch. The code currently expects that if we have an
aggregate function and a non-aggregate field in the SELECT
then the query is not valid in ONLY_FULL_GROUP_BY mode. I investigated that
the argument to the window function (here column a2 was
not marked as a non-aggregated field). I have fixed this and the query gets
rejected in ONLY_FULL_GROUP_BY mode.


> More input below.
>
> On Wed, Jul 08, 2020 at 02:58:31PM +0530, Varun wrote:
> > revision-id: c776cad5f8ecf2675510deeb55724d7255a52503
> (mariadb-10.4.11-267-gc776cad5f8e)
> > parent(s): cc0dca366357651ddb549e31a12b1ecd39c7380e
> > author: Varun Gupta
> > committer: Varun Gupta
> > timestamp: 2020-07-08 14:58:17 +0530
> > message:
> >
> > MDEV-22702: Assertion `!field->is_null()' failed in
> my_decimal::my_decimal
> >
> > With implicit grouping with window functions, we need to make sure that
> all the
> > fields inside the window functions are nullable as any non-aggregated
> field can
> > produce a NULL value.
>
> ...
> > diff --git a/sql/sql_lex.h b/sql/sql_lex.h
> > index 0983cea44d0..16a0b3b08a6 100644
> > --- a/sql/sql_lex.h
> > +++ b/sql/sql_lex.h
> > @@ -1510,6 +1510,8 @@ class st_select_lex: public st_select_lex_node
> >    }
> >
> >    bool have_window_funcs() const { return (window_funcs.elements !=0); }
> > +  uint32 get_number_of_window_funcs() const
> > +  { return (uint32)window_funcs.elements; }
>
> Why use uint32 when n_sum_items is uint, and elements is also uint?
> I suggest just keep using uint.
>

Changed.

>
> >    ORDER *find_common_window_func_partition_fields(THD *thd);
> >
> >    bool cond_pushdown_is_allowed() const
> > diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> > index 0ca5ab23288..05463efe9a5 100644
> > --- a/sql/sql_select.cc
> > +++ b/sql/sql_select.cc
> > @@ -1201,6 +1201,18 @@ JOIN::prepare(TABLE_LIST *tables_init,
> >          break;
> >        }
> >      }
> > +    /*
> > +      If the query has a window function with an aggregate function,
> > +      then also we have a mix of elements with and without grouping.
> > +      Window function can be in the ORDER BY clause too so the check
> > +      is made separately.
>
> Wording could be better.
> I don't understand what did you mean to say about ORDER BY. can you
> elaborate?
>

What i meant here was that window functions can be present both in the
SELECT LIST and ORDER BY clause, so a separate check is made
here. Just above this we iterate over the SELECT LIST for non-aggregate
fields and aggregate functions. If the window function is present
in the ORDER BY clause iterating over the SELECT LIST would not have helped
that is the check is done separately.

But looks like we don't do this check for aggregate functions in the ORDER
BY clause.
Example:
   SELECT a FROM t1 ORDER BY sum(a)
this does not set mixed_implicit_grouping to true. But looks like it is ok
if it is not set. We are just going to send a row with all NULL values to
the client. With window functions this is an issue because we need to
execute the window function for the one row in the output with implicit
grouping


>
> > +      Window function is inherited from Item_sum so each window
> function is
> > +      also registered as a sum item, so need to check that we have an
> > +      explicit aggregate function also in the query.
>
> what's "explicit aggregate" ?  A regular aggregate function (i.e. not a
> window
> function)?
>
Yes explicit aggregate means aggregate function here.

>
> > +    */
> > +    if (select_lex->have_window_funcs() &&
> > +        select_lex->get_number_of_window_funcs() <
> select_lex->n_sum_items)
> > +      mixed_implicit_grouping= true;
> >    }
> >
> >    table_count= select_lex->leaf_tables.elements;
>
> BR
>  Sergei
> --
> Sergei Petrunia, Software Developer
> MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
>
>
>