- developers - lists.mariadb.org

[Maria-developers] Progress of improve IO_CACHE (MDEV-24676)
by Vladisav Kakurin 20 Jul '21

20 Jul '21

To this moment I have implemented medium-grained granularity, and also move IO_CACHE to C++ entitled RingBuffer. In future, it will be fine-grained with slots-pattern. https://github.com/MagHErmit/server/commit/7c066a97403115f6d7702049b741d466…

1 0

Re: [Maria-developers] [Commits] 6f1628b: MDEV-25565 Crash on 2-nd execution of SP/PS for query calculating window functions
by Vicențiu Ciorbaru 20 Jul '21

20 Jul '21

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

1 0

Re: [Maria-developers] 19aebbd1b89: MDEV-16481: set global system_versioning_asof=sf() crashes in specific case
by Sergei Golubchik 19 Jul '21

19 Jul '21

Hi, Nikita! On Jul 12, Nikita Malyavin wrote: > revision-id: 19aebbd1b89 (mariadb-10.3.26-129-g19aebbd1b89) > parent(s): 10c163e4a18 > author: Nikita Malyavin <nikita.malyavin(a)mariadb.com> > committer: Nikita Malyavin <nikita.malyavin(a)mariadb.com> > timestamp: 2021-04-06 21:32:43 +0300 > message: > > MDEV-16481: set global system_versioning_asof=sf() crashes in specific case > > * sys_vars.h: add `MYSQL_TIME` field to `set_var::save_result` > * sys_vars.ic: get rid of calling `var->value->get_date()` from `Sys_var_vers_asof::update()` > * versioning.sysvars: add test; remove double warning > > diff --git a/sql/set_var.h b/sql/set_var.h > index 12e025e4696..6351fb5b666 100644 > --- a/sql/set_var.h > +++ b/sql/set_var.h > @@ -296,6 +296,7 @@ class set_var :public set_var_base > plugin_ref *plugins; ///< for Sys_var_pluginlist > Time_zone *time_zone; ///< for Sys_var_tz > LEX_STRING string_value; ///< for Sys_var_charptr and others > + MYSQL_TIME time; ///< for Sys_var_vers_asof old sizeof(save_result) is 16 bytes, sizeof(MYSQL_TIME) is 40. It's just an observation, set_var isn't a structure that's constantly in memory in large quantities. So, not a problem at all. > const void *ptr; ///< for Sys_var_struct > } save_result; > LEX_CSTRING base; /**< for structured variables, like keycache_name.variable_name */ > diff --git a/sql/sys_vars.ic b/sql/sys_vars.ic > index 8d094d3a1d1..eff6a59e65b 100644 > --- a/sql/sys_vars.ic > +++ b/sql/sys_vars.ic > @@ -2650,50 +2650,47 @@ class Sys_var_vers_asof: public Sys_var_enum > virtual bool do_check(THD *thd, set_var *var) > { > if (!Sys_var_enum::do_check(thd, var)) > + { > + var->save_result.time.time_type= MYSQL_TIMESTAMP_NONE; > return false; > - MYSQL_TIME ltime; > - bool res= var->value->get_date(&ltime, TIME_NO_ZERO_IN_DATE|TIME_NO_ZERO_DATE); > - if (!res) > + } > + bool res= var->value->get_date(&var->save_result.time, > + TIME_NO_ZERO_IN_DATE | TIME_NO_ZERO_DATE); > + if (res) > { > - var->save_result.ulonglong_value= SYSTEM_TIME_AS_OF; > + var->save_result.time.time_type= MYSQL_TIMESTAMP_ERROR; > } > return res; > } > > private: > - bool update(set_var *var, vers_asof_timestamp_t &out, system_variables& pool) > + bool update(THD *thd, set_var *var, vers_asof_timestamp_t *out) > { > - bool res= false; > - uint error; > - MYSQL_TIME ltime; > - out.type= static_cast<enum_var_type>(var->save_result.ulonglong_value); > - if (out.type == SYSTEM_TIME_AS_OF) > + MYSQL_TIME &ltime= var->save_result.time; > + uint error= 0; > + > + if (var->value && ltime.time_type >= 0) // any valid value is ok > { > - if (var->value) > - { > - res= var->value->get_date(&ltime, TIME_NO_ZERO_IN_DATE > - | TIME_NO_ZERO_DATE); > - out.unix_time= pool.time_zone->TIME_to_gmt_sec(&ltime, &error); > - out.second_part= ltime.second_part; > - res|= (error != 0); > - } > - else // set DEFAULT from global var > - { > - out.type= SYSTEM_TIME_UNSPECIFIED; > - res= false; > - } > + out->type = SYSTEM_TIME_AS_OF; > + out->unix_time = thd->variables.time_zone->TIME_to_gmt_sec(&ltime, > + &error); why are you doing it here? You could've converted in ::check(), and as a bonus you wouldn't need MYSQL_TIME in the save_result. And what would you do if TIME_to_gmt_sec would fail? ::update() isn't expected to fail, ::check() should guarantee that as much as possible. > + out->second_part= ltime.second_part; > } > - return res; > + else // DEFAULT is set > + { > + out->type= SYSTEM_TIME_UNSPECIFIED; > + } > + return (error != 0); > } > > public: > virtual bool global_update(THD *thd, set_var *var) > { > - return update(var, global_var(vers_asof_timestamp_t), thd->variables); > + return update(thd, var, &global_var(vers_asof_timestamp_t)); > } > virtual bool session_update(THD *thd, set_var *var) > { > - return update(var, session_var(thd, vers_asof_timestamp_t), thd->variables); > + return update(thd, var, &session_var(thd, vers_asof_timestamp_t)); > } > > private: > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] a11756e4fba: MDEV-23178: Qualified asterisk not supported in INSERT .. RETURNING
by Sergei Golubchik 16 Jul '21

16 Jul '21

Hi, Rucha! On Jul 15, Rucha Deodhar wrote: > revision-id: a11756e4fba (mariadb-10.5.11-27-ga11756e4fba) > parent(s): f0f47cbca18 > author: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > committer: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > timestamp: 2021-07-05 20:14:19 +0530 > message: > > MDEV-23178: Qualified asterisk not supported in INSERT .. RETURNING > > Analysis: When we have INSERT/REPLACE returning with qualified asterisk in the > RETURNING clause, '*' is not resolved properly because of wrong context. > context->table_list is NULL or has incorrect table because context->table_list > has tables from the FROM clause. > Fix: If filling fields instead of '*' for qualified asterisk in RETURNING, > use first_name_resolution_table for correct resolution of item. please, add a couple of tests with a qualified asterisk, where it's qualified _not_ by the table you're inserting into. Like INSERT INTO t1(id1,val1) VALUES(14,'m') RETURNING t2.* not very interesting case, an obvious error ^^^ INSERT INTO t2 SELECT t1.* FROM t1 WHERE id1=2 RETURNING t1.*; but what will happen here? ^^^ > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 1476d708d75..60d14c6dbea 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -8032,12 +8032,13 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, > else treat natural joins as leaves and do not iterate over their underlying > tables. > */ > - for (TABLE_LIST *tables= (table_name ? context->table_list : > - context->first_name_resolution_table); > + TABLE_LIST *tables= returning_field || !table_name ? context->first_name_resolution_table : > + context->table_list; > + for (; > tables; > - tables= (table_name ? tables->next_local : > - tables->next_name_resolution_table) > + tables= returning_field || !table_name ? tables->next_name_resolution_table : > + tables->next_local > ) > { > Field *field; > TABLE *table= tables->table; 1. please also update the comment to mention RETURNING. 2. Could you apply the following patch as a separate commit before your fix: ============================ --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8098,12 +8098,14 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, else treat natural joins as leaves and do not iterate over their underlying tables. */ - for (TABLE_LIST *tables= (table_name ? context->table_list : - context->first_name_resolution_table); - tables; - tables= (table_name ? tables->next_local : - tables->next_name_resolution_table) - ) + TABLE_LIST *first= context->first_name_resolution_table; + TABLE_LIST *TABLE_LIST::* next= &TABLE_LIST::next_name_resolution_table; + if (table_name) + { + first= context->table_list; + next= &TABLE_LIST::next_local; + } + for (TABLE_LIST *tables= first; tables; tables= tables->*next) { Field *field; TABLE *table= tables->table; ============================ it'll remove the condition from inside the loop, will put it in one place only. And will make your change simpler and clearer. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 70252733d41: MDEV-25908: -e does not work for my_print_defaults
by Sergei Golubchik 15 Jul '21

15 Jul '21

Hi, Rucha! On Jul 15, Rucha Deodhar wrote: > revision-id: 70252733d41 (mariadb-10.5.11-35-g70252733d41) > parent(s): f88d130e710 > author: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > committer: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > timestamp: 2021-07-14 22:55:21 +0530 > message: > > MDEV-25908: -e does not work for my_print_defaults > > Analysis: get_defaults_options() rewrites the value for my_defaults_file, > my_defaults_extra_file and my_defaults_group_suffix to 0. So the config file > can't be read. > Fix: Let handle_options() handle --defaults* option and create a new command > line to pass it to load_defaults(). > > --- > extra/my_print_defaults.c | 37 +++++++++++++++++----- > mysql-test/main/my_print_defaults.result | 35 ++++++++++++++++++++- > mysql-test/main/my_print_defaults.test | 54 +++++++++++++++++++++++++++++++- > mysql-test/main/mysqldump.result | 1 + > 4 files changed, 117 insertions(+), 10 deletions(-) > > diff --git a/extra/my_print_defaults.c b/extra/my_print_defaults.c > index 1dec667949f..409ef273b44 100644 > --- a/extra/my_print_defaults.c > +++ b/extra/my_print_defaults.c > @@ -141,28 +141,49 @@ static int get_options(int *argc,char ***argv) > return 0; > } > > +static char *make_args(const char *s1, const char *s2) > +{ > + char *s= malloc(strlen(s1) + strlen(s2) + 1); > + strmov(strmov(s, s1), s2); > + return s; > +} > > int main(int argc, char **argv) > { > - int count, error, args_used; > - char **load_default_groups= 0, *tmp_arguments[6]; > + int count= 0, error, no_defaults= 0; > + char **load_default_groups= 0; > char **argument, **arguments, **org_argv; > int nargs, i= 0; > MY_INIT(argv[0]); > > org_argv= argv; > - args_used= get_defaults_options(argv); > - > - /* Copy defaults-xxx arguments & program name */ > - count=args_used; > - arguments= tmp_arguments; > - memcpy((char*) arguments, (char*) org_argv, count * sizeof(*org_argv)); > + if (*argv && !strcmp(*argv, "--no-defaults")) > + { > + argv++; > + ++count; > + no_defaults= 1; > + } > + /* Copy program name and --no-defaults if present*/ > + memcpy((char*) arguments, (char*) org_argv, (++count)*sizeof(*org_argv)); isn't this wrong? `arguments` is uninitialized at this point. > arguments[count]= 0; > > /* Check out the args */ > if (get_options(&argc,&argv)) > cleanup_and_exit(1); > > +--echo # Testing -e > +--exec $MYSQL_MY_PRINT_DEFAULTS -e $MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld mysql.server > +--echo #Testing --defaults-extra-file > +--exec $MYSQL_MY_PRINT_DEFAULTS --defaults-extra-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld mysql.server > + > +--echo # Testing -c option > +--exec $MYSQL_MY_PRINT_DEFAULTS -c $MYSQLTEST_VARDIR/tmp/tmp2.cnf --mysqld mysql.server > +--echo # Testing --defaults-file > +--exec $MYSQL_MY_PRINT_DEFAULTS --defaults-file=$MYSQLTEST_VARDIR/tmp/tmp2.cnf --mysqld mysql.server > + > +--echo # Testing -g option > +--exec $MYSQL_MY_PRINT_DEFAULTS -c $MYSQLTEST_VARDIR/tmp/tmp3.cnf --mysqld mysql.server -g .1 > +--echo # Testing --defaults-group-suffix > +--exec $MYSQL_MY_PRINT_DEFAULTS -c $MYSQLTEST_VARDIR/tmp/tmp3.cnf --mysqld mysql.server --defaults-group-suffix=.1 you don't seem have a --no-defaults test Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] da70f190279: MDEV-25985 Spider handle ">=" as ">" in some cases
by Sergei Golubchik 13 Jul '21

13 Jul '21

Hi, Nayuta! The code change is good, no comments about it at all. But why did you create a new suite spider/bugfix and all those files in it? I know that some later spider version has it. Still, I'd expect it'd be much simpler and a smaller change to drop the test file into the exising spider suite. On Jul 12, Nayuta Yanagisawa wrote: > revision-id: da70f190279 (mariadb-10.2.39-44-gda70f190279) > parent(s): 99f700a820e > author: Nayuta Yanagisawa <nayuta.yanagisawa(a)hey.com> > committer: Nayuta Yanagisawa <nayuta.yanagisawa(a)hey.com> > timestamp: 2021-07-08 14:12:45 +0000 > message: > > MDEV-25985 Spider handle ">=" as ">" in some cases > > The function spider_db_append_key_where_internal() converts > HA_READ_AFTER_KEY to '>'. The conversion seems to be correct for > single-column indexes because HA_READ_AFTER_KEY means "read the > key after the provided value." > > However, how about multi-column indexes? Assume that there is a > multi-column index on c1 and c2 and we search with the condition > 'c1 >= 100 AND c2 > 200'. The key_range.flag corresponds to the > search condition could be HA_READ_AFTER_KEY. In such a case, > we could not simply convert HA_READ_AFTER_KEY to '>'. > > The correct conversion is to convert HA_READ_AFTER_KEY to '>' > only for the last column in key_part_map and to convert > HA_READ_AFTER_KEY to '>=' for the other column. > > The similar discussion also applies to the conversion from > key_range.flag to a sign of inequality. > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] a6e96ce3c96: MDEV-21916: COM_STMT_BULK_EXECUTE with RETURNING insert wrong values
by Sergei Golubchik 12 Jul '21

12 Jul '21

Hi, Oleksandr! ok to push! On Jul 12, Oleksandr Byelkin wrote: > revision-id: a6e96ce3c96 (mariadb-10.5.4-666-ga6e96ce3c96) > parent(s): f4425d3a3db > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2021-06-09 10:43:24 +0200 > message: > > MDEV-21916: COM_STMT_BULK_EXECUTE with RETURNING insert wrong values > > The problem is that array binding uses net buffer to read parameters for each > execution while each execiting with RETURNING write in the same buffer. > > Solution is to allocate new net buffer to avoid changing buffer we are reading > from. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] b1881e07797: MDEV-20131 Assertion `!pk->has_virtual ()' failed in instant_alter_column_possible
by Sergei Golubchik 12 Jul '21

12 Jul '21

Hi, Sachin! On Jul 12, Sachin Kumar wrote: > revision-id: b1881e07797 (mariadb-10.4.20-9-gb1881e07797) > parent(s): 8a2b4d531dc > author: Sachin Kumar <sachin.setiya(a)mariadb.com> > committer: Sachin Kumar <sachin.setiya(a)mariadb.com> > timestamp: 2021-06-11 14:26:19 +0100 > message: > > MDEV-20131 Assertion `!pk->has_virtual ()' failed in instant_alter_column_possible > > Problem:- Primary key on long unique columns is not allowed but when > `CREATE TABLE t2 (a TEXT, PRIMARY KEY(a(1871))) ENGINE=InnoDB;` > is executed with innodb_page_size=8k or 4k, primary key on DB_ROW_HASH_1 column > is created. Reason of this is in mysql_prepare_create_table we check against > max_key_part_length() to see whether key_part length is more then engine supported > key_part length , and if it is true error is thrown for primary key and long > unique index is created for unique key. But in this case max_key_part_length() > returns 3072 which is more the max_key_length() , which later in code triggers > creating of long unique index. > > Solution:- max_supported_key_part_length() should return according to innodb > page size. > > diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc > index ce2e4b6990f..e5e864bffc5 100644 > --- a/storage/innobase/handler/ha_innodb.cc > +++ b/storage/innobase/handler/ha_innodb.cc > @@ -6537,14 +6537,21 @@ ha_innobase::clone( > DBUG_RETURN(new_handler); > } > > - > -uint > -ha_innobase::max_supported_key_part_length() const > -/*==============================================*/ > -{ > - /* A table format specific index column length check will be performed > - at ha_innobase::add_index() and row_create_index_for_mysql() */ > - return(REC_VERSION_56_MAX_INDEX_COL_LEN); > +uint ha_innobase::max_supported_key_part_length() const > +{ > + /* A table format specific index column length check will be performed > + at ha_innobase::add_index() and row_create_index_for_mysql() */ > + /* FIXME: rewrite this as well as ha_innobase::max_supported_key_length() > + using an API that considers the PRIMARY KEY as well as secondary index > + metadata and the ROW_FORMAT and KEY_BLOCK_SIZE */ > + switch (srv_page_size) { > + case 4096: > + return 1173; > + case 8192: > + return 1536; > + default: > + return REC_VERSION_56_MAX_INDEX_COL_LEN; > + } dunno, why would you duplicate the whole switch logic? can you just - return(REC_VERSION_56_MAX_INDEX_COL_LEN); - return MY_MIN(max_supported_key_length(), REC_VERSION_56_MAX_INDEX_COL_LEN); > } > > /******************************************************************//** > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 85186734823: MDEV-24248: my_print_defaults is not taking all the values when using -e
by Sergei Golubchik 12 Jul '21

12 Jul '21

Hi, Rucha! On Jul 12, Rucha Deodhar wrote: > revision-id: 85186734823 (mariadb-10.5.4-665-g85186734823) > parent(s): bf5c050fd28 > author: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > committer: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > timestamp: 2021-06-14 10:27:13 +0530 > message: > > MDEV-24248: my_print_defaults is not taking all the values when using -e > option which is called from mysql.server (extra_args). > > Fix: change mysql.server script to use --defaults-extra-file instead of -e > > diff --git a/mysql-test/main/my_print_defaults.test b/mysql-test/main/my_print_defaults.test > --- /dev/null > +++ b/mysql-test/main/my_print_defaults.test > @@ -0,0 +1,29 @@ > +--echo # > +--echo # Beginning of 10.5 Test > +--echo # > +--echo # > +--echo # MDEV-24248: my_print_defaults is not taking all the values when using > +--echo # -e option which is called from mysql.server (extra_args). > +--echo # > + > +--echo # checking for mysql.server > + > +--write_file $MYSQLTEST_VARDIR/tmp/tmp1.cnf > + > +[mariadb-10.5] don't use the version number in the test - you're forcing some poor soul to update the test on merges for every new version. > +key_buffer_size=20M > +max_allowed_packet=250M > +table_open_cache=1000 > +table_definition_cache=2000 > +read_buffer_size=1M > +thread_cache_size=8 > +max_connections=1024 > +long_query_time=60 > +slow_query_log=1 > +EOF > + > +--exec $MYSQL_MY_PRINT_DEFAULTS --defaults-extra-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld mysql.server > + > +--remove_file $MYSQLTEST_VARDIR/tmp/tmp1.cnf > + > +--echo # End of 10.5 test > diff --git a/support-files/mysql.server.sh b/support-files/mysql.server.sh > index cefb3225e9e..8504e4def4b 100644 > --- a/support-files/mysql.server.sh > +++ b/support-files/mysql.server.sh > @@ -200,11 +200,11 @@ su_kill() { > extra_args="" > if test -r "$basedir/my.cnf" > then > - extra_args="-e $basedir/my.cnf" > + extra_args="--defaults-file-name=$basedir/my.cnf" eh. -e is --defaults-extra-file same below. > else > if test -r "$datadir/my.cnf" > then > - extra_args="-e $datadir/my.cnf" > + extra_args="--defaults-file-name=$datadir/my.cnf" > fi > fi > > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] b29ec61724d: MDEV-25906: SIGSEGV in flush_tables_with_read_lock on FTWRL or FTFE | SIGSEGV in ha_maria::extra
by Sergei Golubchik 01 Jul '21

01 Jul '21

Hi, Oleksandr! On Jul 01, Oleksandr Byelkin wrote: > revision-id: b29ec61724d (mariadb-10.6.2-41-gb29ec61724d) > parent(s): 83684fc9a4d > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2021-07-01 11:20:29 +0200 > message: > > MDEV-25906: SIGSEGV in flush_tables_with_read_lock on FTWRL or FTFE | SIGSEGV in ha_maria::extra > > Fixed check of derived tables of all kinds (view, derived, information schema). > > diff --git a/sql/sql_reload.cc b/sql/sql_reload.cc > index 0fa2fa10df8..e41da9680b0 100644 > --- a/sql/sql_reload.cc > +++ b/sql/sql_reload.cc > @@ -592,10 +592,19 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) > for (TABLE_LIST *table_list= all_tables; table_list; > table_list= table_list->next_global) > { > - if (!(table_list->is_view() || > - table_list->table->file->ha_table_flags() & HA_CAN_EXPORT)) > + /* > + We allow all deriveds and view here, but prohibit information_schema > + tables (explicitly) and performance_schema tables (implicetly: > + engine do not suport export) > + */ > + if (table_list->is_non_derived() && > + (!table_list->is_base_table() || > + !(table_list->table->file->ha_table_flags() & HA_CAN_EXPORT))) > { > - my_error(ER_ILLEGAL_HA, MYF(0),table_list->table->file->table_type(), > + my_error(ER_ILLEGAL_HA, MYF(0), > + (table_list->is_base_table() ? > + table_list->table->file->table_type(): > + "information_schema"), I'd say if you want to detect I_S tables, you should test for table_list->schema_table. It's not clear what your condition actually does. And there is no such engine "information_schema", so the message is misleading. In fact, this should be access denied. For both. Like: mysqltest: At line 12: query 'flush table information_schema.collations for export' failed: ER_DBACCESS_DENIED_ERROR (1044): Access denied for user 'foo'@'localhost' to database 'information_schema' this is from a test file that only does flush table information_schema.collations for export. > table_list->db.str, table_list->table_name.str); > goto error_reset_bits; > } > @@ -607,7 +616,8 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) > for (auto table_list= all_tables; table_list; > table_list= table_list->next_global) > { > - if (!table_list->is_view() && > + if (table_list->table && why do you check for table_list->table ? Can it happen that table_list->table == NULL and table_list->is_base_table() == true? > + table_list->is_base_table() && > table_list->table->file->extra(HA_EXTRA_FLUSH)) > goto error_reset_bits; > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0