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@mariadb.com> 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@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits