maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #12796
Re: [Commits] 6f1628b: MDEV-25565 Crash on 2-nd execution of SP/PS for query calculating window functions
Hi Igor!
The patch looks good, however I suggest the following changes to make it
smaller and easier to read & maintain.
1. You do not need to have special if statements for save_partition_list
and save_order_list when you do the cleanup.
2. You do not need to have special code that saves the partition list and
the order list *only* when it is overwritten.
You can instead always set save_partition_list to be equal to
partition_list in the Window_spec's constructor. Same for save_order_list
and order_list.
Then, during cleanup, you can always execute
order_list= save_order_list;
partition_list= save_partition_list;
The advantage of this change is that it removes all the references to
save_xxxx_list in sql_window.cc. You then only have a simple cleanup
function sql_union.cc.
3. I would prefer to name the variables "initial_order_list" and
"initial_partition_list".
The patch then becomes:
diff --git a/sql/sql_union.cc b/sql/sql_union.cc
index e5648e6989b..54a33bb7863 100644
--- a/sql/sql_union.cc
+++ b/sql/sql_union.cc
@@ -30,6 +30,7 @@
#include "filesort.h" // filesort_free_buffers
#include "sql_view.h"
#include "sql_cte.h"
+#include "item_windowfunc.h"
bool mysql_union(THD *thd, LEX *lex, select_result *result,
SELECT_LEX_UNIT *unit, ulong setup_tables_done_option)
@@ -1551,6 +1552,21 @@ static void cleanup_order(ORDER *order)
}
+static void cleanup_window_funcs(List<Item_window_func> &win_funcs)
+{
+ List_iterator_fast<Item_window_func> it(win_funcs);
+ Item_window_func *win_func;
+ while ((win_func= it++))
+ {
+ Window_spec *win_spec= win_func->window_spec;
+ if (!win_spec)
+ continue;
+ win_spec->partition_list= win_spec->initial_partition_list;
+ win_spec->order_list= win_spec->initial_order_list;
+ }
+}
+
+
bool st_select_lex::cleanup()
{
bool error= FALSE;
@@ -1559,6 +1575,8 @@ bool st_select_lex::cleanup()
cleanup_order(order_list.first);
cleanup_order(group_list.first);
+ cleanup_window_funcs(window_funcs);
+
if (join)
{
List_iterator<TABLE_LIST> ti(leaf_tables);
diff --git a/sql/sql_window.h b/sql/sql_window.h
index e0c1563e5bb..cf84e26c822 100644
--- a/sql/sql_window.h
+++ b/sql/sql_window.h
@@ -99,8 +99,10 @@ class Window_spec : public Sql_alloc
LEX_STRING *window_ref;
SQL_I_List<ORDER> *partition_list;
+ SQL_I_List<ORDER> *initial_partition_list;
SQL_I_List<ORDER> *order_list;
+ SQL_I_List<ORDER> *initial_order_list;
Window_frame *window_frame;
@@ -111,7 +113,8 @@ class Window_spec : public Sql_alloc
SQL_I_List<ORDER> *ord_list,
Window_frame *win_frame)
: window_names_are_checked(false), window_ref(win_ref),
- partition_list(part_list), order_list(ord_list),
+ partition_list(part_list), initial_partition_list(part_list),
+ order_list(ord_list), initial_order_list(ord_list),
window_frame(win_frame), referenced_win_spec(NULL) {}
virtual char *name() { return NULL; }
On Sat, 10 Jul 2021 at 04:57, IgorBabaev <igor@xxxxxxxxxxx> wrote:
> revision-id: 6f1628b917d365ecfc9c4c9951011613f4212592
> (mariadb-10.2.31-1048-g6f1628b)
> parent(s): fb0b28932ce82903f2fcfb690a71bff52355507f
> author: Igor Babaev
> committer: Igor Babaev
> timestamp: 2021-07-09 18:56:34 -0700
> message:
>
> MDEV-25565 Crash on 2-nd execution of SP/PS for query calculating window
> functions
> from view
>
> A crash of the server happened when executing a stored procedure whose the
> only query calculated window functions over a mergeable view specified
> as a select from non-mergeable view. The crash could be reproduced if
> the window specifications of the window functions were identical and both
> contained PARTITION lists and ORDER BY lists. A crash also happened on
> the second execution of the prepared statement created for such query.
> If to use derived tables or CTE instead of views the problem still
> manifests itself crashing the server.
>
> When optimizing the window specifications of a window function the
> server can substitute the partition lists and the order lists for
> the corresponding lists from another window specification in the case
> when the lists are identical. This substitution is not permanent and should
> be rolled back before the second execution. It was not done and this
> ultimately led to a crash when resolving the column names at the second
> execution of SP/PS.
>
> ---
> mysql-test/r/win.result | 287
> ++++++++++++++++++++++++++++++++++++++++++++++++
> mysql-test/t/win.test | 147 +++++++++++++++++++++++++
> sql/sql_union.cc | 26 +++++
> sql/sql_window.cc | 12 ++
> sql/sql_window.h | 5 +-
> 5 files changed, 476 insertions(+), 1 deletion(-)
>
> diff --git a/mysql-test/r/win.result b/mysql-test/r/win.result
> index 8a31dcc..bc017ea 100644
> --- a/mysql-test/r/win.result
> +++ b/mysql-test/r/win.result
> @@ -3911,5 +3911,292 @@ sum(i) over () IN ( SELECT 1 FROM t1 a)
> 0
> DROP TABLE t1;
> #
> +# MDEV-25565: 2-nd call of SP with SELECT from view / derived table / CTE
> +# returning the result of calculation of 2 window
> +# functions that use the same window specification
> +#
> +create table t1 (a int);
> +insert into t1 values (3), (7), (1), (7), (1), (1), (3), (1), (5);
> +create view v2 as select a from t1 group by a;
> +create view v1 as select * from v2;
> +create procedure sp1() select v1.a,
> +sum(v1.a) over (partition by v1.a order by v1.a) as k,
> +avg(v1.a) over (partition by v1.a order by v1.a) as m
> +from v1;
> +call sp1();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +call sp1();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +prepare stmt from "select v1.a,
> +sum(v1.a) over (partition by v1.a order by v1.a) as k,
> +avg(v1.a) over (partition by v1.a order by v1.a) as m
> +from v1";
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +deallocate prepare stmt;
> +create procedure sp2() select * from
> +( select dt1.a,
> +sum(dt1.a) over (partition by dt1.a order by dt1.a) as k,
> +avg(dt1.a) over (partition by dt1.a order by dt1.a) as m
> +from (select * from v2) as dt1
> +) as dt;
> +call sp2();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +call sp2();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +prepare stmt from "select * from
> +( select dt1.a,
> +sum(dt1.a) over (partition by dt1.a order by dt1.a) as k,
> +avg(dt1.a) over (partition by dt1.a order by dt1.a) as m
> +from (select * from v2) as dt1
> +) as dt";
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +deallocate prepare stmt;
> +create procedure sp3() select * from
> +( select dt1.a,
> +sum(dt1.a) over (partition by dt1.a order by dt1.a) as k,
> +avg(dt1.a) over (partition by dt1.a order by dt1.a) as m
> +from ( select * from (select * from t1 group by a) as dt2 ) as dt1
> +) as dt;
> +call sp3();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +call sp3();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +prepare stmt from "select * from
> +( select dt1.a,
> +sum(dt1.a) over (partition by dt1.a order by dt1.a) as k,
> +avg(dt1.a) over (partition by dt1.a order by dt1.a) as m
> +from ( select * from (select * from t1 group by a) as dt2 ) as dt1
> +) as dt";
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +deallocate prepare stmt;
> +create procedure sp4() with cte1 as (select * from (select * from t1
> group by a) as dt2),
> +cte as
> +( select cte1.a,
> +sum(cte1.a) over (partition by cte1.a order by cte1.a) as k,
> +avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> +from cte1 )
> +select * from cte;
> +call sp4();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +call sp4();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +prepare stmt from "with cte1 as (select * from (select * from t1 group by
> a) as dt2),
> +cte as
> +( select cte1.a,
> +sum(cte1.a) over (partition by cte1.a order by cte1.a) as k,
> +avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> +from cte1 )
> +select * from cte";
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +deallocate prepare stmt;
> +create procedure sp5() with cte1 as (select * from v2),
> +cte as
> +( select cte1.a,
> +sum(cte1.a) over (partition by cte1.a order by cte1.a) as k,
> +avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> +from cte1 )
> +select * from cte;
> +call sp5();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +call sp5();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +prepare stmt from "with cte1 as (select * from v2),
> +cte as
> +( select cte1.a,
> +sum(cte1.a) over (partition by cte1.a order by cte1.a) as k,
> +avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> +from cte1 )
> +select * from cte";
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +deallocate prepare stmt;
> +create procedure sp6() with
> +cte1 as (with cte2 as (select * from t1 group by a) select * from cte2),
> +cte as
> +( select cte1.a,
> +sum(cte1.a) over (partition by cte1.a order by cte1.a) as k,
> +avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> +from cte1 )
> +select * from cte;
> +call sp6();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +call sp6();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +prepare stmt from "with
> +cte1 as (with cte2 as (select * from t1 group by a) select * from cte2),
> +cte as
> +( select cte1.a,
> +sum(cte1.a) over (partition by cte1.a order by cte1.a) as k,
> +avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> +from cte1 )
> +select * from cte";
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +deallocate prepare stmt;
> +create procedure sp7() with
> +cte2 as (select * from v1),
> +cte1 as (select * from cte2),
> +cte as
> +( select cte1.a,
> +sum(cte1.a) over (partition by cte1.a order by cte1.a) as k,
> +avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> +from cte1 )
> +select * from cte;
> +call sp7();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +call sp7();
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +prepare stmt from "with
> +cte2 as (select * from v1),
> +cte1 as (select * from cte2),
> +cte as
> +( select cte1.a,
> +sum(cte1.a) over (partition by cte1.a order by cte1.a) as k,
> +avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> +from cte1 )
> +select * from cte";
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +execute stmt;
> +a k m
> +1 1 1.0000
> +3 3 3.0000
> +5 5 5.0000
> +7 7 7.0000
> +deallocate prepare stmt;
> +drop procedure sp1;
> +drop procedure sp2;
> +drop procedure sp3;
> +drop procedure sp4;
> +drop procedure sp5;
> +drop procedure sp6;
> +drop procedure sp7;
> +drop view v1,v2;
> +drop table t1;
> +#
> # End of 10.2 tests
> #
> diff --git a/mysql-test/t/win.test b/mysql-test/t/win.test
> index c07a81f..72e789d 100644
> --- a/mysql-test/t/win.test
> +++ b/mysql-test/t/win.test
> @@ -2557,5 +2557,152 @@ SELECT sum(i) over () IN ( SELECT 1 FROM t1 a)
> FROM t1;
> DROP TABLE t1;
>
> --echo #
> +--echo # MDEV-25565: 2-nd call of SP with SELECT from view / derived
> table / CTE
> +--echo # returning the result of calculation of 2 window
> +--echo # functions that use the same window specification
> +--echo #
> +
> +create table t1 (a int);
> +insert into t1 values (3), (7), (1), (7), (1), (1), (3), (1), (5);
> +
> +create view v2 as select a from t1 group by a;
> +create view v1 as select * from v2;
> +
> +let $q1=
> +select v1.a,
> + sum(v1.a) over (partition by v1.a order by v1.a) as k,
> + avg(v1.a) over (partition by v1.a order by v1.a) as m
> +from v1;
> +
> +eval create procedure sp1() $q1;
> +call sp1();
> +call sp1();
> +
> +eval prepare stmt from "$q1";
> +execute stmt;
> +execute stmt;
> +deallocate prepare stmt;
> +
> +let $q2=
> +select * from
> + ( select dt1.a,
> + sum(dt1.a) over (partition by dt1.a order by dt1.a) as k,
> + avg(dt1.a) over (partition by dt1.a order by dt1.a) as m
> + from (select * from v2) as dt1
> + ) as dt;
> +
> +eval create procedure sp2() $q2;
> +call sp2();
> +call sp2();
> +
> +eval prepare stmt from "$q2";
> +execute stmt;
> +execute stmt;
> +deallocate prepare stmt;
> +
> +let $q3=
> +select * from
> + ( select dt1.a,
> + sum(dt1.a) over (partition by dt1.a order by dt1.a) as k,
> + avg(dt1.a) over (partition by dt1.a order by dt1.a) as m
> + from ( select * from (select * from t1 group by a) as dt2 ) as dt1
> + ) as dt;
> +
> +eval create procedure sp3() $q3;
> +call sp3();
> +call sp3();
> +
> +eval prepare stmt from "$q3";
> +execute stmt;
> +execute stmt;
> +deallocate prepare stmt;
> +
> +let $q4=
> +with cte1 as (select * from (select * from t1 group by a) as dt2),
> + cte as
> + ( select cte1.a,
> + sum(cte1.a) over (partition by cte1.a order by cte1.a) as
> k,
> + avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> + from cte1 )
> +select * from cte;
> +
> +eval create procedure sp4() $q4;
> +call sp4();
> +call sp4();
> +
> +eval prepare stmt from "$q4";
> +execute stmt;
> +execute stmt;
> +deallocate prepare stmt;
> +
> +let $q5=
> +with cte1 as (select * from v2),
> + cte as
> + ( select cte1.a,
> + sum(cte1.a) over (partition by cte1.a order by cte1.a) as
> k,
> + avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> + from cte1 )
> +select * from cte;
> +
> +eval create procedure sp5() $q5;
> +call sp5();
> +call sp5();
> +
> +eval prepare stmt from "$q5";
> +execute stmt;
> +execute stmt;
> +deallocate prepare stmt;
> +
> +let $q6=
> +with
> +cte1 as (with cte2 as (select * from t1 group by a) select * from cte2),
> + cte as
> + ( select cte1.a,
> + sum(cte1.a) over (partition by cte1.a order by cte1.a) as
> k,
> + avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> + from cte1 )
> +select * from cte;
> +
> +eval create procedure sp6() $q6;
> +call sp6();
> +call sp6();
> +
> +eval prepare stmt from "$q6";
> +execute stmt;
> +execute stmt;
> +deallocate prepare stmt;
> +
> +let $q7=
> +with
> + cte2 as (select * from v1),
> + cte1 as (select * from cte2),
> + cte as
> + ( select cte1.a,
> + sum(cte1.a) over (partition by cte1.a order by cte1.a) as
> k,
> + avg(cte1.a) over (partition by cte1.a order by cte1.a) as m
> + from cte1 )
> +select * from cte;
> +
> +eval create procedure sp7() $q7;
> +call sp7();
> +call sp7();
> +
> +eval prepare stmt from "$q7";
> +execute stmt;
> +execute stmt;
> +deallocate prepare stmt;
> +
> +
> +drop procedure sp1;
> +drop procedure sp2;
> +drop procedure sp3;
> +drop procedure sp4;
> +drop procedure sp5;
> +drop procedure sp6;
> +drop procedure sp7;
> +drop view v1,v2;
> +drop table t1;
> +
> +--echo #
> --echo # End of 10.2 tests
> --echo #
> diff --git a/sql/sql_union.cc b/sql/sql_union.cc
> index 7baedfb..f3c90b8 100644
> --- a/sql/sql_union.cc
> +++ b/sql/sql_union.cc
> @@ -30,6 +30,7 @@
> #include "filesort.h" // filesort_free_buffers
> #include "sql_view.h"
> #include "sql_cte.h"
> +#include "item_windowfunc.h"
>
> bool mysql_union(THD *thd, LEX *lex, select_result *result,
> SELECT_LEX_UNIT *unit, ulong setup_tables_done_option)
> @@ -1550,6 +1551,29 @@ static void cleanup_order(ORDER *order)
> }
>
>
> +static void cleanup_window_funcs(List<Item_window_func> &win_funcs)
> +{
> + List_iterator_fast<Item_window_func> it(win_funcs);
> + Item_window_func *win_func;
> + while ((win_func= it++))
> + {
> + Window_spec *win_spec= win_func->window_spec;
> + if (!win_spec)
> + continue;
> + if (win_spec->save_partition_list)
> + {
> + win_spec->partition_list= win_spec->save_partition_list;
> + win_spec->save_partition_list= NULL;
> + }
> + if (win_spec->save_order_list)
> + {
> + win_spec->order_list= win_spec->save_order_list;
> + win_spec->save_order_list= NULL;
> + }
> + }
> +}
> +
> +
> bool st_select_lex::cleanup()
> {
> bool error= FALSE;
> @@ -1558,6 +1582,8 @@ bool st_select_lex::cleanup()
> cleanup_order(order_list.first);
> cleanup_order(group_list.first);
>
> + cleanup_window_funcs(window_funcs);
> +
> if (join)
> {
> List_iterator<TABLE_LIST> ti(leaf_tables);
> diff --git a/sql/sql_window.cc b/sql/sql_window.cc
> index 612c6e6..3ef751b 100644
> --- a/sql/sql_window.cc
> +++ b/sql/sql_window.cc
> @@ -479,9 +479,15 @@ int
> compare_window_funcs_by_window_specs(Item_window_func *win_func1,
> Let's use only one of the lists.
> */
> if (!win_spec1->name() && win_spec2->name())
> + {
> + win_spec1->save_partition_list= win_spec1->partition_list;
> win_spec1->partition_list= win_spec2->partition_list;
> + }
> else
> + {
> + win_spec2->save_partition_list= win_spec2->partition_list;
> win_spec2->partition_list= win_spec1->partition_list;
> + }
>
> cmp= compare_order_lists(win_spec1->order_list,
> win_spec2->order_list);
> @@ -494,9 +500,15 @@ int
> compare_window_funcs_by_window_specs(Item_window_func *win_func1,
> Let's use only one of the lists.
> */
> if (!win_spec1->name() && win_spec2->name())
> + {
> + win_spec1->save_order_list= win_spec2->order_list;
> win_spec1->order_list= win_spec2->order_list;
> + }
> else
> + {
> + win_spec1->save_order_list= win_spec2->order_list;
> win_spec2->order_list= win_spec1->order_list;
> + }
>
> cmp= compare_window_frames(win_spec1->window_frame,
> win_spec2->window_frame);
> diff --git a/sql/sql_window.h b/sql/sql_window.h
> index e0c1563..417d0bc 100644
> --- a/sql/sql_window.h
> +++ b/sql/sql_window.h
> @@ -99,8 +99,10 @@ class Window_spec : public Sql_alloc
> LEX_STRING *window_ref;
>
> SQL_I_List<ORDER> *partition_list;
> + SQL_I_List<ORDER> *save_partition_list;
>
> SQL_I_List<ORDER> *order_list;
> + SQL_I_List<ORDER> *save_order_list;
>
> Window_frame *window_frame;
>
> @@ -111,7 +113,8 @@ class Window_spec : public Sql_alloc
> SQL_I_List<ORDER> *ord_list,
> Window_frame *win_frame)
> : window_names_are_checked(false), window_ref(win_ref),
> - partition_list(part_list), order_list(ord_list),
> + partition_list(part_list), save_partition_list(NULL),
> + order_list(ord_list), save_order_list(NULL),
> window_frame(win_frame), referenced_win_spec(NULL) {}
>
> virtual char *name() { return NULL; }
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
