Re: [Maria-developers] [Commits] c776cad5f8e: MDEV-22702: Assertion `!field->is_null()' failed in my_decimal::my_decimal
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). 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.
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?
+ 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)?
+ */ + 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
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
Hi Sergey, On Thu, Aug 6, 2020 at 4:31 PM Sergey Petrunia <sergey@mariadb.com> wrote: 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
participants (2)
-
Sergey Petrunia -
varun gupta