From 794bebf9ee18de4138c4d2c93853d49ff3af7f12 Mon Sep 17 00:00:00 2001 From: anel Date: Fri, 1 Apr 2022 07:17:43 -0700 Subject: [PATCH 1/8] Use proper pid namespace Problem: ============== By testing `pgrep` with `--ns` option, introduced with MDEV-21331, commit fb7c1b9415c9a8b0dc2e86ae44f0e7a2634e5d7e, I noted that: a) `--ns` cannot use more than single PID. b) `--ns` is returning the processes of the namespace to which supplied PID belongs to. So by that sense command `pgrep -x --ns $$ mysqld` will always return an error and skip checking of the existing PID of the server. Solution: ============== Suggested solution is to add `--nslist pid`, since `--ns` needs to know in which namespace type it should look for. See `pgrep --help` for different namespace types. Note also that this works *only* if script is run as a `root` (we have that case here). Current PR is a part of: 1. MDEV-21331: sync preinst and postrm script 2. MDEV-15718: check for exact mysqld process This commit: a) fixes fb7c1b9415c9a8b0dc2e86ae44f0e7a2634e5d7e b) Closes PR #2068 (obsolete) c) Closes PR #2069 (obsolete) Thanks Faustin Lammler for testing and verifying Reviewed by <> --- debian/mariadb-server-10.2.postrm | 5 +++++ debian/mariadb-server-10.2.preinst | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/debian/mariadb-server-10.2.postrm b/debian/mariadb-server-10.2.postrm index f8a95df54ea..1ead4098906 100644 --- a/debian/mariadb-server-10.2.postrm +++ b/debian/mariadb-server-10.2.postrm @@ -11,6 +11,11 @@ MYADMIN="/usr/bin/mysqladmin --defaults-file=/etc/mysql/debian.cnf" # do it himself. No database directories should be removed while the server # is running! stop_server() { + # Return immediately if there are no mysql processes running + # as there is no point in trying to shutdown in that case. + # Compatibility with versions that ran 'mariadbd' + if ! pgrep -x --nslist pid --ns $$ "mysqld|mariadbd" > /dev/null; then return; fi + set +e if [ -x /usr/sbin/invoke-rc.d ]; then invoke-rc.d mysql stop diff --git a/debian/mariadb-server-10.2.preinst b/debian/mariadb-server-10.2.preinst index 161d9caee6e..13dda71998a 100644 --- a/debian/mariadb-server-10.2.preinst +++ b/debian/mariadb-server-10.2.preinst @@ -22,10 +22,11 @@ mysql_upgradedir=/var/lib/mysql-upgrade # is running! Another mysqld in e.g. a different chroot is fine for us. stop_server() { if [ ! -x /etc/init.d/mysql ]; then return; fi - - # Return immediately if there are no mysql processes running + # Return immediately if there are no mysql processes running on a host + # (leave containerized processes with the same name in other namespaces) # as there is no point in trying to shutdown in that case. - if ! pgrep --ns $$ mysqld > /dev/null; then return; fi + # Compatibility with versions that ran 'mariadbd' + if ! pgrep -x --nslist pid --ns $$ "mysqld|mariadbd" > /dev/null; then return; fi set +e if [ -x /usr/sbin/invoke-rc.d ]; then From ba4927e520190bbad763bb5260ae154f29a61231 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Tue, 3 May 2022 14:06:27 +0300 Subject: [PATCH 2/8] MDEV-19398: Assertion `item1->type() == Item::FIELD_ITEM ... Window Functions code tries to minimize the number of times it needs to sort the select's resultset by finding "compatible" OVER (PARTITION BY ... ORDER BY ...) clauses. This employs compare_order_elements(). That function assumed that the order expressions are Item_field-derived objects (that refer to a temp.table). But this is not always the case: one can construct queries order expressions are arbitrary item expressions. Add handling for such expressions: sort them according to the window specification they appeared in. This means we cannot detect that two compatible PARTITION BY clauses that use expressions can share the sorting step. But at least we won't crash. --- mysql-test/r/win.result | 42 ++++++++++++++++++++++++++ mysql-test/t/win.test | 33 +++++++++++++++++++++ sql/sql_parse.cc | 2 ++ sql/sql_window.cc | 65 ++++++++++++++++++++++++++++++++++------- sql/sql_window.h | 7 +++++ 5 files changed, 139 insertions(+), 10 deletions(-) diff --git a/mysql-test/r/win.result b/mysql-test/r/win.result index 1ddafbd4f8f..b4d918eb437 100644 --- a/mysql-test/r/win.result +++ b/mysql-test/r/win.result @@ -4238,5 +4238,47 @@ SELECT 1 UNION SELECT a FROM t1 ORDER BY (row_number() over ()); ERROR HY000: Expression #1 of ORDER BY contains aggregate function and applies to a UNION DROP TABLE t1; # +# MDEV-19398: Assertion `item1->type() == Item::FIELD_ITEM && +# item2->type() == Item::FIELD_ITEM' failed in compare_order_elements +# +CREATE TABLE t1 ( id varchar(10)); +INSERT INTO t1 values (1),(2),(3); +SELECT +dense_rank() over (ORDER BY avg(1)+3), +rank() over (ORDER BY avg(1)) +FROM t1 +GROUP BY nullif(id, 15532); +dense_rank() over (ORDER BY avg(1)+3) rank() over (ORDER BY avg(1)) +1 1 +1 1 +1 1 +SELECT +dense_rank() over (ORDER BY avg(1)), +rank() over (ORDER BY avg(1)) +FROM t1 +GROUP BY nullif(id, 15532); +dense_rank() over (ORDER BY avg(1)) rank() over (ORDER BY avg(1)) +1 1 +1 1 +1 1 +drop table t1; +CREATE TABLE t1 ( a char(25), b text); +INSERT INTO t1 VALUES ('foo','bar'); +SELECT +SUM(b) OVER (PARTITION BY a), +ROW_NUMBER() OVER (PARTITION BY b) +FROM t1 +GROUP BY +LEFT((SYSDATE()), 'foo') +WITH ROLLUP; +SUM(b) OVER (PARTITION BY a) ROW_NUMBER() OVER (PARTITION BY b) +NULL 1 +NULL 1 +Warnings: +Warning 1292 Truncated incorrect INTEGER value: 'foo' +Warning 1292 Truncated incorrect INTEGER value: 'foo' +drop table t1; +# +# # End of 10.2 tests # diff --git a/mysql-test/t/win.test b/mysql-test/t/win.test index 37f09a6e850..ba1008afa60 100644 --- a/mysql-test/t/win.test +++ b/mysql-test/t/win.test @@ -2740,6 +2740,39 @@ INSERT INTO t1 VALUES (1),(1),(1),(1),(1),(2),(2),(2),(2),(2),(2); SELECT 1 UNION SELECT a FROM t1 ORDER BY (row_number() over ()); DROP TABLE t1; +--echo # +--echo # MDEV-19398: Assertion `item1->type() == Item::FIELD_ITEM && +--echo # item2->type() == Item::FIELD_ITEM' failed in compare_order_elements +--echo # +CREATE TABLE t1 ( id varchar(10)); +INSERT INTO t1 values (1),(2),(3); + +SELECT + dense_rank() over (ORDER BY avg(1)+3), + rank() over (ORDER BY avg(1)) +FROM t1 +GROUP BY nullif(id, 15532); + +SELECT + dense_rank() over (ORDER BY avg(1)), + rank() over (ORDER BY avg(1)) +FROM t1 +GROUP BY nullif(id, 15532); +drop table t1; + +CREATE TABLE t1 ( a char(25), b text); +INSERT INTO t1 VALUES ('foo','bar'); + +SELECT + SUM(b) OVER (PARTITION BY a), + ROW_NUMBER() OVER (PARTITION BY b) +FROM t1 +GROUP BY + LEFT((SYSDATE()), 'foo') +WITH ROLLUP; +drop table t1; + +--echo # --echo # --echo # End of 10.2 tests --echo # diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 989ca0c8803..457849a7569 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -8624,6 +8624,7 @@ bool st_select_lex::add_window_def(THD *thd, fields_in_window_functions+= win_part_list_ptr->elements + win_order_list_ptr->elements; } + win_def->win_spec_number= window_specs.elements; return (win_def == NULL || window_specs.push_back(win_def)); } @@ -8651,6 +8652,7 @@ bool st_select_lex::add_window_spec(THD *thd, win_order_list_ptr->elements; } thd->lex->win_spec= win_spec; + win_spec->win_spec_number= window_specs.elements; return (win_spec == NULL || window_specs.push_back(win_spec)); } diff --git a/sql/sql_window.cc b/sql/sql_window.cc index 3ef751bc5b9..8afdaa1e6da 100644 --- a/sql/sql_window.cc +++ b/sql/sql_window.cc @@ -312,15 +312,49 @@ setup_windows(THD *thd, Ref_ptr_array ref_pointer_array, TABLE_LIST *tables, #define CMP_GT 2 // Greater then static -int compare_order_elements(ORDER *ord1, ORDER *ord2) +int compare_order_elements(ORDER *ord1, int weight1, + ORDER *ord2, int weight2) { if (*ord1->item == *ord2->item && ord1->direction == ord2->direction) return CMP_EQ; Item *item1= (*ord1->item)->real_item(); Item *item2= (*ord2->item)->real_item(); - DBUG_ASSERT(item1->type() == Item::FIELD_ITEM && - item2->type() == Item::FIELD_ITEM); - ptrdiff_t cmp= ((Item_field *) item1)->field - ((Item_field *) item2)->field; + + bool item1_field= (item1->type() == Item::FIELD_ITEM); + bool item2_field= (item2->type() == Item::FIELD_ITEM); + + ptrdiff_t cmp; + if (item1_field && item2_field) + { + DBUG_ASSERT(((Item_field *) item1)->field->table == + ((Item_field *) item2)->field->table); + cmp= ((Item_field *) item1)->field->field_index - + ((Item_field *) item2)->field->field_index; + } + else if (item1_field && !item2_field) + return CMP_LT; + else if (!item1_field && item2_field) + return CMP_LT; + else + { + /* + Ok, item1_field==NULL and item2_field==NULL. + We're not able to compare Item expressions. Order them according to + their passed "weight" (which comes from Window_spec::win_spec_number): + */ + if (weight1 != weight2) + cmp= weight1 - weight2; + else + { + /* + The weight is the same. That is, the elements come from the same + window specification... This shouldn't happen. + */ + DBUG_ASSERT(0); + cmp= item1 - item2; + } + } + if (cmp == 0) { if (ord1->direction == ord2->direction) @@ -333,7 +367,9 @@ int compare_order_elements(ORDER *ord1, ORDER *ord2) static int compare_order_lists(SQL_I_List *part_list1, - SQL_I_List *part_list2) + int spec_number1, + SQL_I_List *part_list2, + int spec_number2) { if (part_list1 == part_list2) return CMP_EQ; @@ -358,7 +394,8 @@ int compare_order_lists(SQL_I_List *part_list1, if (!elem1 || !elem2) break; - if ((cmp= compare_order_elements(elem1, elem2))) + if ((cmp= compare_order_elements(elem1, spec_number1, + elem2, spec_number2))) return cmp; } if (elem1) @@ -453,7 +490,9 @@ int compare_window_spec_joined_lists(Window_spec *win_spec1, win_spec1->join_partition_and_order_lists(); win_spec2->join_partition_and_order_lists(); int cmp= compare_order_lists(win_spec1->partition_list, - win_spec2->partition_list); + win_spec1->win_spec_number, + win_spec2->partition_list, + win_spec2->win_spec_number); win_spec1->disjoin_partition_and_order_lists(); win_spec2->disjoin_partition_and_order_lists(); return cmp; @@ -471,7 +510,9 @@ int compare_window_funcs_by_window_specs(Item_window_func *win_func1, if (win_spec1 == win_spec2) return CMP_EQ; cmp= compare_order_lists(win_spec1->partition_list, - win_spec2->partition_list); + win_spec1->win_spec_number, + win_spec2->partition_list, + win_spec2->win_spec_number); if (cmp == CMP_EQ) { /* @@ -490,7 +531,9 @@ int compare_window_funcs_by_window_specs(Item_window_func *win_func1, } cmp= compare_order_lists(win_spec1->order_list, - win_spec2->order_list); + win_spec1->win_spec_number, + win_spec2->order_list, + win_spec2->win_spec_number); if (cmp != CMP_EQ) return cmp; @@ -587,7 +630,9 @@ void order_window_funcs_by_window_specs(List *win_func_list) int cmp; if (win_spec_prev->partition_list == win_spec_curr->partition_list) cmp= compare_order_lists(win_spec_prev->order_list, - win_spec_curr->order_list); + win_spec_prev->win_spec_number, + win_spec_curr->order_list, + win_spec_curr->win_spec_number); else cmp= compare_window_spec_joined_lists(win_spec_prev, win_spec_curr); if (!(CMP_LT_C <= cmp && cmp <= CMP_GT_C)) diff --git a/sql/sql_window.h b/sql/sql_window.h index 417d0bca12c..b29038fc374 100644 --- a/sql/sql_window.h +++ b/sql/sql_window.h @@ -108,6 +108,13 @@ class Window_spec : public Sql_alloc Window_spec *referenced_win_spec; + /* + Window_spec objects are numbered by the number of their appearance in the + query. This is used by compare_order_elements() to provide a predictable + ordering of PARTITION/ORDER BY clauses. + */ + int win_spec_number; + Window_spec(LEX_STRING *win_ref, SQL_I_List *part_list, SQL_I_List *ord_list, From 8dbfaa2aa4d6158f81bba3f5a46d683912b06868 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Wed, 4 May 2022 12:24:14 +0300 Subject: [PATCH 3/8] MDEV-28437: Assertion `!eliminated' failed in Item_subselect::exec (This is the assert that was added in fix for MDEV-26047) Table elimination may remove an ON expression from an outer join. However SELECT_LEX::update_used_tables() will still call item->walk(&Item::eval_not_null_tables) for eliminated expressions. If the subquery is constant and cheap Item_cond_and will attempt to evaluate it, which will trigger an assert. The fix is not to call update_used_tables() or eval_not_null_tables() for ON expressions that were eliminated. --- mysql-test/r/subselect_innodb.result | 11 +++++++++++ mysql-test/t/subselect_innodb.test | 14 ++++++++++++++ sql/sql_lex.cc | 9 ++++++--- sql/sql_select.cc | 2 +- sql/sql_select.h | 2 ++ 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/subselect_innodb.result b/mysql-test/r/subselect_innodb.result index 4796245f8e3..fd75cce00b2 100644 --- a/mysql-test/r/subselect_innodb.result +++ b/mysql-test/r/subselect_innodb.result @@ -661,3 +661,14 @@ group by (select a),(select 1) ); 1 drop table t1; +# +# MDEV-28437: Assertion `!eliminated' failed in Item_subselect::exec +# +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (b INT PRIMARY KEY) ENGINE=InnoDB; +INSERT INTO t1 VALUES (3),(4); +SELECT 1 IN (SELECT a FROM t1 LEFT JOIN t2 ON (a = b AND EXISTS (SELECT * FROM t1))); +1 IN (SELECT a FROM t1 LEFT JOIN t2 ON (a = b AND EXISTS (SELECT * FROM t1))) +1 +drop table t1,t2; diff --git a/mysql-test/t/subselect_innodb.test b/mysql-test/t/subselect_innodb.test index f42ac514d53..5d8f3dcc1b5 100644 --- a/mysql-test/t/subselect_innodb.test +++ b/mysql-test/t/subselect_innodb.test @@ -655,3 +655,17 @@ select 1 from t1 where not exists --enable_warnings drop table t1; +--echo # +--echo # MDEV-28437: Assertion `!eliminated' failed in Item_subselect::exec +--echo # +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (b INT PRIMARY KEY) ENGINE=InnoDB; +INSERT INTO t1 VALUES (3),(4); + +SELECT 1 IN (SELECT a FROM t1 LEFT JOIN t2 ON (a = b AND EXISTS (SELECT * FROM t1))); + +drop table t1,t2; + +# End of 10.2 tests + diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 22ee8801e3a..8e718f2a942 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -4229,7 +4229,7 @@ void SELECT_LEX::update_used_tables() } } while ((embedding= embedding->embedding)); - if (tl->on_expr) + if (tl->on_expr && !is_eliminated_table(join->eliminated_tables, tl)) { tl->on_expr->update_used_tables(); tl->on_expr->walk(&Item::eval_not_null_tables, 0, NULL); @@ -4253,8 +4253,11 @@ void SELECT_LEX::update_used_tables() if (embedding->on_expr && embedding->nested_join->join_list.head() == tl) { - embedding->on_expr->update_used_tables(); - embedding->on_expr->walk(&Item::eval_not_null_tables, 0, NULL); + if (!is_eliminated_table(join->eliminated_tables, embedding)) + { + embedding->on_expr->update_used_tables(); + embedding->on_expr->walk(&Item::eval_not_null_tables, 0, NULL); + } } tl= embedding; embedding= tl->embedding; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index e9d81417ee6..82792bbc723 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -25495,7 +25495,7 @@ static void print_table_array(THD *thd, too) */ -static bool is_eliminated_table(table_map eliminated_tables, TABLE_LIST *tbl) +bool is_eliminated_table(table_map eliminated_tables, TABLE_LIST *tbl) { return eliminated_tables && ((tbl->table && (tbl->table->map & eliminated_tables)) || diff --git a/sql/sql_select.h b/sql/sql_select.h index f41c0df5ad8..8b61645710e 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -2330,4 +2330,6 @@ int create_sort_index(THD *thd, JOIN *join, JOIN_TAB *tab, Filesort *fsort); JOIN_TAB *first_explain_order_tab(JOIN* join); JOIN_TAB *next_explain_order_tab(JOIN* join, JOIN_TAB* tab); +bool is_eliminated_table(table_map eliminated_tables, TABLE_LIST *tbl); + #endif /* SQL_SELECT_INCLUDED */ From 84e32eff5b050b69649db942de5a74f1b3d24e6d Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Thu, 5 May 2022 18:58:00 +0300 Subject: [PATCH 4/8] MDEV-28437: Assertion `!eliminated' failed: Part #2 In SELECT_LEX::update_used_tables(), do not run the loop setting tl->table->maybe_null when tl is an eliminated table (Rationale: First, with current table elimination, tl already has maybe_null=1. Second, one should not care what flags eliminated tables had) --- sql/sql_lex.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 8e718f2a942..6191e174298 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -4219,16 +4219,20 @@ void SELECT_LEX::update_used_tables() while ((tl= ti++)) { TABLE_LIST *embedding= tl; - do + if (!is_eliminated_table(join->eliminated_tables, tl)) { - bool maybe_null; - if ((maybe_null= MY_TEST(embedding->outer_join))) + do { - tl->table->maybe_null= maybe_null; - break; + bool maybe_null; + if ((maybe_null= MY_TEST(embedding->outer_join))) + { + tl->table->maybe_null= maybe_null; + break; + } } + while ((embedding= embedding->embedding)); } - while ((embedding= embedding->embedding)); + if (tl->on_expr && !is_eliminated_table(join->eliminated_tables, tl)) { tl->on_expr->update_used_tables(); From 20ae4816bba712a3faa0110c973e197d92f43b42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 6 May 2022 09:30:17 +0300 Subject: [PATCH 5/8] MDEV-28478: INSERT into SPATIAL INDEX in TEMPORARY table writes log row_ins_sec_index_entry_low(): If a separate mini-transaction is needed to adjust the minimum bounding rectangle (MBR) in the parent page, we must disable redo logging if the table is a temporary table. For temporary tables, no log is supposed to be written, because the temporary tablespace will be reinitialized on server restart. rtr_update_mbr_field(): Plug a memory leak. --- .../suite/innodb_gis/r/rtree_split.result | 7 ------- .../suite/innodb_gis/r/rtree_temporary.result | 14 ++++++++++++++ mysql-test/suite/innodb_gis/t/rtree_split.test | 8 -------- .../suite/innodb_gis/t/rtree_temporary.test | 18 ++++++++++++++++++ storage/innobase/gis/gis0rtree.cc | 3 ++- storage/innobase/row/row0ins.cc | 10 +++++++--- 6 files changed, 41 insertions(+), 19 deletions(-) create mode 100644 mysql-test/suite/innodb_gis/r/rtree_temporary.result create mode 100644 mysql-test/suite/innodb_gis/t/rtree_temporary.test diff --git a/mysql-test/suite/innodb_gis/r/rtree_split.result b/mysql-test/suite/innodb_gis/r/rtree_split.result index 2d6e8a1dfbe..df88960ba3d 100644 --- a/mysql-test/suite/innodb_gis/r/rtree_split.result +++ b/mysql-test/suite/innodb_gis/r/rtree_split.result @@ -61,10 +61,3 @@ select count(*) from t1 where MBRWithin(t1.c2, @g1); count(*) 57344 drop table t1; -# -# MDEV-27417 Spatial index tries to update -# change buffer bookkeeping page -# -CREATE TEMPORARY TABLE t1 (c POINT NOT NULL, SPATIAL(c)) ENGINE=InnoDB; -INSERT INTO t1 SELECT PointFromText('POINT(0 0)') FROM seq_1_to_366; -DROP TABLE t1; diff --git a/mysql-test/suite/innodb_gis/r/rtree_temporary.result b/mysql-test/suite/innodb_gis/r/rtree_temporary.result new file mode 100644 index 00000000000..5ce02c881cc --- /dev/null +++ b/mysql-test/suite/innodb_gis/r/rtree_temporary.result @@ -0,0 +1,14 @@ +# +# MDEV-27417 Spatial index tries to update +# change buffer bookkeeping page +# +CREATE TEMPORARY TABLE t1 (c POINT NOT NULL, SPATIAL(c)) ENGINE=InnoDB; +INSERT INTO t1 SELECT PointFromText('POINT(0 0)') FROM seq_1_to_366; +DROP TABLE t1; +# +# MDEV-28478 Assertion mtr->get_log_mode() == MTR_LOG_NO_REDO +# +CREATE TEMPORARY TABLE t1 (c POINT NOT NULL,SPATIAL (c)) ENGINE=InnoDB; +INSERT INTO t1 SELECT POINT(0,0) FROM seq_1_to_366; +INSERT INTO t1 VALUES (POINT(1e-270,1e-130)); +DROP TABLE t1; diff --git a/mysql-test/suite/innodb_gis/t/rtree_split.test b/mysql-test/suite/innodb_gis/t/rtree_split.test index dd46d1ecc4d..af626dba6b7 100644 --- a/mysql-test/suite/innodb_gis/t/rtree_split.test +++ b/mysql-test/suite/innodb_gis/t/rtree_split.test @@ -72,11 +72,3 @@ select count(*) from t1 where MBRWithin(t1.c2, @g1); # Clean up. drop table t1; - ---echo # ---echo # MDEV-27417 Spatial index tries to update ---echo # change buffer bookkeeping page ---echo # -CREATE TEMPORARY TABLE t1 (c POINT NOT NULL, SPATIAL(c)) ENGINE=InnoDB; -INSERT INTO t1 SELECT PointFromText('POINT(0 0)') FROM seq_1_to_366; -DROP TABLE t1; diff --git a/mysql-test/suite/innodb_gis/t/rtree_temporary.test b/mysql-test/suite/innodb_gis/t/rtree_temporary.test new file mode 100644 index 00000000000..5c4df251970 --- /dev/null +++ b/mysql-test/suite/innodb_gis/t/rtree_temporary.test @@ -0,0 +1,18 @@ +--source include/have_innodb.inc +--source include/have_sequence.inc + +--echo # +--echo # MDEV-27417 Spatial index tries to update +--echo # change buffer bookkeeping page +--echo # +CREATE TEMPORARY TABLE t1 (c POINT NOT NULL, SPATIAL(c)) ENGINE=InnoDB; +INSERT INTO t1 SELECT PointFromText('POINT(0 0)') FROM seq_1_to_366; +DROP TABLE t1; + +--echo # +--echo # MDEV-28478 Assertion mtr->get_log_mode() == MTR_LOG_NO_REDO +--echo # +CREATE TEMPORARY TABLE t1 (c POINT NOT NULL,SPATIAL (c)) ENGINE=InnoDB; +INSERT INTO t1 SELECT POINT(0,0) FROM seq_1_to_366; +INSERT INTO t1 VALUES (POINT(1e-270,1e-130)); +DROP TABLE t1; diff --git a/storage/innobase/gis/gis0rtree.cc b/storage/innobase/gis/gis0rtree.cc index 50071bcfae4..b18642b0e3c 100644 --- a/storage/innobase/gis/gis0rtree.cc +++ b/storage/innobase/gis/gis0rtree.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2019, 2020, MariaDB Corporation. +Copyright (c) 2019, 2022, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -376,6 +376,7 @@ rtr_update_mbr_field( if (!rtr_update_mbr_field_in_place(index, rec, offsets, mbr, mtr)) { + mem_heap_free(heap); return(false); } diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc index cbe6577c02a..0c0214b009e 100644 --- a/storage/innobase/row/row0ins.cc +++ b/storage/innobase/row/row0ins.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2016, 2021, MariaDB Corporation. +Copyright (c) 2016, 2022, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -2900,8 +2900,12 @@ row_ins_sec_index_entry_low( rtr_init_rtr_info(&rtr_info, false, &cursor, index, false); rtr_info_update_btr(&cursor, &rtr_info); - mtr_start(&mtr); - mtr.set_named_space(index->space); + mtr.start(); + if (index->table->is_temporary()) { + mtr.set_log_mode(MTR_LOG_NO_REDO); + } else { + mtr.set_named_space(index->space); + } search_mode &= ulint(~BTR_MODIFY_LEAF); search_mode |= BTR_MODIFY_TREE; err = btr_cur_search_to_nth_level( From 624cb9735e737ca3392957e2db2171c2957cf282 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Fri, 6 May 2022 10:34:27 +0300 Subject: [PATCH 6/8] Update test results after fix for MDEV-19398 --- .../encryption/r/tempfiles_encrypted.result | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/mysql-test/suite/encryption/r/tempfiles_encrypted.result b/mysql-test/suite/encryption/r/tempfiles_encrypted.result index f7b5081f625..b166d08bb92 100644 --- a/mysql-test/suite/encryption/r/tempfiles_encrypted.result +++ b/mysql-test/suite/encryption/r/tempfiles_encrypted.result @@ -4244,6 +4244,48 @@ SELECT 1 UNION SELECT a FROM t1 ORDER BY (row_number() over ()); ERROR HY000: Expression #1 of ORDER BY contains aggregate function and applies to a UNION DROP TABLE t1; # +# MDEV-19398: Assertion `item1->type() == Item::FIELD_ITEM && +# item2->type() == Item::FIELD_ITEM' failed in compare_order_elements +# +CREATE TABLE t1 ( id varchar(10)); +INSERT INTO t1 values (1),(2),(3); +SELECT +dense_rank() over (ORDER BY avg(1)+3), +rank() over (ORDER BY avg(1)) +FROM t1 +GROUP BY nullif(id, 15532); +dense_rank() over (ORDER BY avg(1)+3) rank() over (ORDER BY avg(1)) +1 1 +1 1 +1 1 +SELECT +dense_rank() over (ORDER BY avg(1)), +rank() over (ORDER BY avg(1)) +FROM t1 +GROUP BY nullif(id, 15532); +dense_rank() over (ORDER BY avg(1)) rank() over (ORDER BY avg(1)) +1 1 +1 1 +1 1 +drop table t1; +CREATE TABLE t1 ( a char(25), b text); +INSERT INTO t1 VALUES ('foo','bar'); +SELECT +SUM(b) OVER (PARTITION BY a), +ROW_NUMBER() OVER (PARTITION BY b) +FROM t1 +GROUP BY +LEFT((SYSDATE()), 'foo') +WITH ROLLUP; +SUM(b) OVER (PARTITION BY a) ROW_NUMBER() OVER (PARTITION BY b) +NULL 1 +NULL 1 +Warnings: +Warning 1292 Truncated incorrect INTEGER value: 'foo' +Warning 1292 Truncated incorrect INTEGER value: 'foo' +drop table t1; +# +# # End of 10.2 tests # # From 141ab971d8d31968ac7104e71801c6ec75638af3 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Wed, 4 May 2022 19:51:26 +0200 Subject: [PATCH 7/8] MDEV-28402 ASAN heap-use-after-free in create_tmp_table, Assertion `l_offset >= 0 && table->s->rec_buff_length - l_offset > 0' Make default() function follow Item_field and use get_tmp_table_item() for change_to_use_tmp_fields(). --- mysql-test/r/default.result | 18 ++++++++++++++++++ mysql-test/t/default.test | 17 +++++++++++++++++ sql/sql_select.cc | 8 +++++--- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/default.result b/mysql-test/r/default.result index 0c91e6b7e08..1c792983638 100644 --- a/mysql-test/r/default.result +++ b/mysql-test/r/default.result @@ -3413,4 +3413,22 @@ SELECT 1 FROM t1 GROUP BY DEFAULT(pk); 1 1 DROP TABLE t1; +# +# MDEV-28402: ASAN heap-use-after-free in create_tmp_table, +# Assertion `l_offset >= 0 && table->s->rec_buff_length - l_offset > 0' +# +CREATE TABLE t (a INT, KEY (a)); +INSERT INTO t VALUES (1),(2); +SELECT DISTINCT DEFAULT(a), CASE a WHEN 0 THEN 1 ELSE 2 END FROM t GROUP BY a WITH ROLLUP; +DEFAULT(a) CASE a WHEN 0 THEN 1 ELSE 2 END +NULL 2 +DROP TABLE t; +CREATE TABLE t (a INT, KEY (a)); +INSERT INTO t VALUES (1),(2); +CREATE ALGORITHM=TEMPTABLE VIEW v AS SELECT * FROM t; +SELECT DISTINCT DEFAULT(a), CASE a WHEN 0 THEN 1 ELSE 2 END FROM v GROUP BY a WITH ROLLUP; +DEFAULT(a) CASE a WHEN 0 THEN 1 ELSE 2 END +NULL 2 +DROP TABLE t; +DROP VIEW v; # end of 10.2 test diff --git a/mysql-test/t/default.test b/mysql-test/t/default.test index e0233a3929a..3bc373d0313 100644 --- a/mysql-test/t/default.test +++ b/mysql-test/t/default.test @@ -2125,4 +2125,21 @@ CREATE TABLE t1 (pk varchar(36) DEFAULT uuid()); INSERT INTO t1 VALUES (),(); SELECT 1 FROM t1 GROUP BY DEFAULT(pk); DROP TABLE t1; + + +--echo # +--echo # MDEV-28402: ASAN heap-use-after-free in create_tmp_table, +--echo # Assertion `l_offset >= 0 && table->s->rec_buff_length - l_offset > 0' +--echo # +CREATE TABLE t (a INT, KEY (a)); +INSERT INTO t VALUES (1),(2); +SELECT DISTINCT DEFAULT(a), CASE a WHEN 0 THEN 1 ELSE 2 END FROM t GROUP BY a WITH ROLLUP; +DROP TABLE t; + +CREATE TABLE t (a INT, KEY (a)); +INSERT INTO t VALUES (1),(2); +CREATE ALGORITHM=TEMPTABLE VIEW v AS SELECT * FROM t; +SELECT DISTINCT DEFAULT(a), CASE a WHEN 0 THEN 1 ELSE 2 END FROM v GROUP BY a WITH ROLLUP; +DROP TABLE t; +DROP VIEW v; --echo # end of 10.2 test diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 82792bbc723..760730d799c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -23727,12 +23727,14 @@ change_to_use_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, for (uint i= 0; (item= it++); i++) { Field *field; - if ((item->with_sum_func && item->type() != Item::SUM_FUNC_ITEM) || + enum Item::Type item_type= item->type(); + if ((item->with_sum_func && item_type != Item::SUM_FUNC_ITEM) || item->with_window_func) item_field= item; - else if (item->type() == Item::FIELD_ITEM) + else if (item_type == Item::FIELD_ITEM || + item_type == Item::DEFAULT_VALUE_ITEM) item_field= item->get_tmp_table_item(thd); - else if (item->type() == Item::FUNC_ITEM && + else if (item_type == Item::FUNC_ITEM && ((Item_func*)item)->functype() == Item_func::SUSERVAR_FUNC) { field= item->get_tmp_table_field(); From a5dc12eefd4bea1c3f77d02c55d0d459b4ae0566 Mon Sep 17 00:00:00 2001 From: Andrei Date: Tue, 3 May 2022 22:38:20 +0300 Subject: [PATCH 8/8] MDEV-28310 Missing binlog data for INSERT .. ON DUPLICATE KEY UPDATE MDEV-21810 MBR: Unexpected "Unsafe statement" warning for unsafe IODKU MDEV-17614 fixes to replication unsafety for INSERT ON DUP KEY UPDATE on two or more unique key table left a flaw. The fixes checked the safety condition per each inserted record with the idea to catch a user-created value to an autoincrement column and when that succeeds the autoincrement column would become the source of unsafety too. It was not expected that after a duplicate error the next record's write_set may become different and the unsafe decision for that specific record will be computed to screw the Query's binlogging state and when @@binlog_format is MIXED nothing gets bin-logged. This case has been already fixed in 10.5.2 by 91ab42a823 that relocated/optimized THD::decide_logging_format_low() out of the record insert loop. The safety decision is computed once and at the right time. Pertinent parts of the commit are cherry-picked. Also a spurious warning about unsafety is removed when MIXED @@binlog_format; original MDEV-17614 test result corrected. The original test of MDEV-17614 is extended and made more readable. --- mysql-test/suite/rpl/r/rpl_iodku,stmt.rdiff | 27 ++++++ mysql-test/suite/rpl/r/rpl_iodku.result | 32 +++++++ mysql-test/suite/rpl/r/rpl_mdev_17614.result | 72 +++++++++++++++ .../suite/rpl/r/rpl_unsafe_statements.result | 5 - mysql-test/suite/rpl/t/rpl_iodku.test | 50 ++++++++++ mysql-test/suite/rpl/t/rpl_mdev_17614.test | 73 ++++++++++++--- .../suite/rpl/t/rpl_unsafe_statements.test | 2 +- sql/sql_class.cc | 91 +++++++++++++------ sql/sql_class.h | 20 ++-- sql/sql_insert.cc | 6 +- 10 files changed, 323 insertions(+), 55 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_iodku,stmt.rdiff create mode 100644 mysql-test/suite/rpl/r/rpl_iodku.result create mode 100644 mysql-test/suite/rpl/t/rpl_iodku.test diff --git a/mysql-test/suite/rpl/r/rpl_iodku,stmt.rdiff b/mysql-test/suite/rpl/r/rpl_iodku,stmt.rdiff new file mode 100644 index 00000000000..e31f1e5d991 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_iodku,stmt.rdiff @@ -0,0 +1,27 @@ +--- r/rpl_iodku.result 2022-05-04 18:51:24.956414404 +0300 ++++ r/rpl_iodku,stmt.reject 2022-05-04 18:51:49.520106231 +0300 +@@ -1,10 +1,15 @@ + include/master-slave.inc + [connection master] ++call mtr.add_suppression("Unsafe statement written to the binary log using statement"); + CREATE TABLE t1 (id INT PRIMARY KEY AUTO_INCREMENT, a INT, b INT, c INT, + UNIQUE (a), UNIQUE (b)) ENGINE=innodb; + INSERT INTO t1 (`a`,`c`) VALUES (1,1), (2,1) ON DUPLICATE KEY UPDATE c = 1; ++Warnings: ++Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe + # UNSAFE + INSERT INTO t1 (`a`,`c`) VALUES (3, 1),(2,1), (1,1) ON DUPLICATE KEY UPDATE c = a * 10 + VALUES(c); ++Warnings: ++Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe + SELECT * from t1; + id a b c + 1 1 NULL 11 +@@ -17,6 +22,8 @@ + INSERT INTO t1 VALUES (1,10,1); + # eligable for the statement format run unsafe warning + INSERT INTO t1 VALUES (2,20,2) ON DUPLICATE KEY UPDATE c = 100; ++Warnings: ++Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe + # not eligable: no warning in the statement format run + INSERT INTO t1 (`a`,`c`) VALUES (3, 1) ON DUPLICATE KEY UPDATE c = 99; + SELECT * from t1; diff --git a/mysql-test/suite/rpl/r/rpl_iodku.result b/mysql-test/suite/rpl/r/rpl_iodku.result new file mode 100644 index 00000000000..55348da1439 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_iodku.result @@ -0,0 +1,32 @@ +include/master-slave.inc +[connection master] +CREATE TABLE t1 (id INT PRIMARY KEY AUTO_INCREMENT, a INT, b INT, c INT, +UNIQUE (a), UNIQUE (b)) ENGINE=innodb; +INSERT INTO t1 (`a`,`c`) VALUES (1,1), (2,1) ON DUPLICATE KEY UPDATE c = 1; +# UNSAFE +INSERT INTO t1 (`a`,`c`) VALUES (3, 1),(2,1), (1,1) ON DUPLICATE KEY UPDATE c = a * 10 + VALUES(c); +SELECT * from t1; +id a b c +1 1 NULL 11 +2 2 NULL 21 +3 3 NULL 1 +connection slave; +include/diff_tables.inc [master:t1,slave:t1] +connection master; +CREATE OR REPLACE TABLE t1 (a INT, b INT, c INT, UNIQUE (a), UNIQUE (b)) ENGINE=innodb; +INSERT INTO t1 VALUES (1,10,1); +# eligable for the statement format run unsafe warning +INSERT INTO t1 VALUES (2,20,2) ON DUPLICATE KEY UPDATE c = 100; +# not eligable: no warning in the statement format run +INSERT INTO t1 (`a`,`c`) VALUES (3, 1) ON DUPLICATE KEY UPDATE c = 99; +SELECT * from t1; +a b c +1 10 1 +2 20 2 +3 NULL 1 +connection slave; +include/diff_tables.inc [master:t1,slave:t1] +connection master; +DROP TABLE t1; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_mdev_17614.result b/mysql-test/suite/rpl/r/rpl_mdev_17614.result index 39057334926..ba077111522 100644 --- a/mysql-test/suite/rpl/r/rpl_mdev_17614.result +++ b/mysql-test/suite/rpl/r/rpl_mdev_17614.result @@ -1,5 +1,6 @@ include/master-slave.inc [connection master] +# Case 1: UNSAFE call mtr.add_suppression("Unsafe statement written to the binary log using statement format"); CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY , b INT, UNIQUE(b), c int) engine=innodb; @@ -37,6 +38,7 @@ drop table t1; connection slave; start slave; include/wait_for_slave_to_start.inc +# Case 2: UNSAFE connection master; CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT, UNIQUE(b), c int) engine=innodb; @@ -45,8 +47,12 @@ connection master; INSERT INTO t1 VALUES (default, 1, 1); BEGIN; INSERT INTO t1 VALUES (default, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe connection master1; INSERT INTO t1 VALUES(default, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe connection master; COMMIT; SELECT * FROM t1; @@ -62,6 +68,7 @@ a b c connection master; drop table t1; connection slave; +# Case 3A: UNSAFE connection master; CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY, b INT, UNIQUE(b), c int, d int ) engine=innodb; @@ -93,6 +100,67 @@ a b c d connection master; drop table t1; connection slave; +# Case 3B: UNSAFE - all column specified. +connection master; +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY, b INT, +UNIQUE(b), c int, d int ) engine=innodb; +connection slave; +connection master; +INSERT INTO t1 VALUES (1, 1, 1, 1); +BEGIN; +INSERT INTO t1 VALUES (2, NULL, 2, 2) ON DUPLICATE KEY UPDATE c=VALUES(c); +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe +connection master1; +INSERT INTO t1 VALUES(3, NULL, 2, 3) ON DUPLICATE KEY UPDATE c=VALUES(c); +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe +connection master; +COMMIT; +SELECT * FROM t1; +a b c d +1 1 1 1 +2 NULL 2 2 +3 NULL 2 3 +connection slave; +#same data as master +SELECT * FROM t1; +a b c d +1 1 1 1 +2 NULL 2 2 +3 NULL 2 3 +connection master; +drop table t1; +connection slave; +# Case 3C: SAFE - only one unique key (PK) specified. +connection master; +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY, b INT, +UNIQUE(b), c int, d int ) engine=innodb; +connection slave; +connection master; +INSERT INTO t1 VALUES (1, 1, 1, 1); +BEGIN; +INSERT INTO t1 (`a`, `c`, `d`) VALUES (2, 2, 2) ON DUPLICATE KEY UPDATE c=99; +connection master1; +INSERT INTO t1 (`a`, `c`, `d`) VALUES(3, 2, 3) ON DUPLICATE KEY UPDATE c=100; +connection master; +COMMIT; +SELECT * FROM t1; +a b c d +1 1 1 1 +2 NULL 2 2 +3 NULL 2 3 +connection slave; +#same data as master +SELECT * FROM t1; +a b c d +1 1 1 1 +2 NULL 2 2 +3 NULL 2 3 +connection master; +drop table t1; +connection slave; +# Case 4: UNSAFE connection master; CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT, UNIQUE(b), c int) engine=innodb; @@ -101,8 +169,12 @@ connection master; INSERT INTO t1 VALUES (1, 1, 1); BEGIN; INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe connection master1; INSERT INTO t1 VALUES(2, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe connection master; COMMIT; SELECT * FROM t1; diff --git a/mysql-test/suite/rpl/r/rpl_unsafe_statements.result b/mysql-test/suite/rpl/r/rpl_unsafe_statements.result index 0ce94ca63d0..ca790f5d148 100644 --- a/mysql-test/suite/rpl/r/rpl_unsafe_statements.result +++ b/mysql-test/suite/rpl/r/rpl_unsafe_statements.result @@ -1,6 +1,5 @@ include/master-slave.inc [connection master] -call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); CREATE TABLE t1(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB; CREATE TABLE t2(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB; CREATE TRIGGER trig1 AFTER INSERT ON t1 @@ -50,13 +49,9 @@ connection master; DROP TABLE t1; CREATE TABLE t1(i INT, j INT, UNIQUE KEY(i), UNIQUE KEY(j)) ENGINE=INNODB; INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1; -Warnings: -Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe START TRANSACTION; LOCK TABLES t1 WRITE; INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1; -Warnings: -Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe UNLOCK TABLES; COMMIT; connection slave; diff --git a/mysql-test/suite/rpl/t/rpl_iodku.test b/mysql-test/suite/rpl/t/rpl_iodku.test new file mode 100644 index 00000000000..815b927c350 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_iodku.test @@ -0,0 +1,50 @@ +--source include/have_innodb.inc +--source include/master-slave.inc + +if (`select @@binlog_format = "statement"`) +{ + call mtr.add_suppression("Unsafe statement written to the binary log using statement"); +} + +## MDEV-28310 loss of binlog event for multi-record IODKU +# Check that the duplicate key error does not cause +# loss of replication event for IODKU that specifies values +# for at least two unique columns per record. +# "Implicit" NULL value of the auto-increment column also counts. + +CREATE TABLE t1 (id INT PRIMARY KEY AUTO_INCREMENT, a INT, b INT, c INT, + UNIQUE (a), UNIQUE (b)) ENGINE=innodb; +INSERT INTO t1 (`a`,`c`) VALUES (1,1), (2,1) ON DUPLICATE KEY UPDATE c = 1; +--echo # UNSAFE +# because of two keys involved: a UK and PK even though implicitly via auto-inc +INSERT INTO t1 (`a`,`c`) VALUES (3, 1),(2,1), (1,1) ON DUPLICATE KEY UPDATE c = a * 10 + VALUES(c); +SELECT * from t1; + +--sync_slave_with_master +--let $diff_tables = master:t1,slave:t1 +--source include/diff_tables.inc + +## MDEV-21810 MBR: Unexpected "Unsafe statement" warning for unsafe IODKU +# Unnecessary unsafe statement warning is not error-logged anymore. + + +--connection master +CREATE OR REPLACE TABLE t1 (a INT, b INT, c INT, UNIQUE (a), UNIQUE (b)) ENGINE=innodb; +INSERT INTO t1 VALUES (1,10,1); +--echo # eligable for the statement format run unsafe warning +INSERT INTO t1 VALUES (2,20,2) ON DUPLICATE KEY UPDATE c = 100; +--echo # not eligable: no warning in the statement format run +INSERT INTO t1 (`a`,`c`) VALUES (3, 1) ON DUPLICATE KEY UPDATE c = 99; +SELECT * from t1; + +--sync_slave_with_master +--let $diff_tables = master:t1,slave:t1 +--source include/diff_tables.inc + +# Cleanup +--connection master +DROP TABLE t1; +--sync_slave_with_master + + +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_mdev_17614.test b/mysql-test/suite/rpl/t/rpl_mdev_17614.test index 9b86c8c15b5..c11aad3305e 100644 --- a/mysql-test/suite/rpl/t/rpl_mdev_17614.test +++ b/mysql-test/suite/rpl/t/rpl_mdev_17614.test @@ -2,15 +2,22 @@ source include/have_debug.inc; source include/have_innodb.inc; -- source include/have_binlog_format_statement.inc source include/master-slave.inc; -# MDEV-17614 -# INSERT on dup key update is replication unsafe -# There can be three case -# 1. 2 unique key, Replication is unsafe. -# 2. 2 unique key , with one auto increment key, Safe to replicate because Innodb will acquire gap lock -# 3. n no of unique keys (n>1) but insert is only in 1 unique key -# 4. 2 unique key , with one auto increment key(but user gives auto inc value), unsafe to replicate +# MDEV-17614 INSERT on dup key update is replication unsafe +# +# The following cases are tested below: +# 1. 2 unique key, replication is UNSAFE +# 2. 2 unique key, with one auto increment key and implicit value to it. +# It is UNSAFE because autoinc column values of being inserted records +# are revealed dynamically, so unknown at the binlog-format decision time +# and hence this pessimistic expectation +# 3. 2 unique keys +# A. insert is only in 1 unique key, still all colums are specified => UNSAFE +# B. both unique keys are specified => UNSAFE +# C. only one unique key is specified => SAFE (motivated by MDEV-28310) +# 4. 2 unique key, with one auto increment key(but user gives auto inc value) => +# UNSAFE to replicate -# Case 1 +--echo # Case 1: UNSAFE call mtr.add_suppression("Unsafe statement written to the binary log using statement format"); CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY , b INT, UNIQUE(b), c int) engine=innodb; @@ -42,7 +49,8 @@ drop table t1; connection slave; start slave; --source include/wait_for_slave_to_start.inc -# Case 2 + +--echo # Case 2: UNSAFE --connection master CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT, UNIQUE(b), c int) engine=innodb; @@ -64,7 +72,7 @@ connection master; drop table t1; --sync_slave_with_master -# Case 3 +--echo # Case 3A: UNSAFE --connection master CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY, b INT, UNIQUE(b), c int, d int ) engine=innodb; @@ -85,7 +93,50 @@ connection master; drop table t1; --sync_slave_with_master -# Case 4 +--echo # Case 3B: UNSAFE - all column specified. +--connection master +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY, b INT, +UNIQUE(b), c int, d int ) engine=innodb; +sync_slave_with_master; +connection master; +INSERT INTO t1 VALUES (1, 1, 1, 1); +BEGIN; +INSERT INTO t1 VALUES (2, NULL, 2, 2) ON DUPLICATE KEY UPDATE c=VALUES(c); + --connection master1 + INSERT INTO t1 VALUES(3, NULL, 2, 3) ON DUPLICATE KEY UPDATE c=VALUES(c); +--connection master +COMMIT; +SELECT * FROM t1; +--sync_slave_with_master +--echo #same data as master +SELECT * FROM t1; +connection master; +drop table t1; +--sync_slave_with_master + + +--echo # Case 3C: SAFE - only one unique key (PK) specified. +--connection master +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY, b INT, +UNIQUE(b), c int, d int ) engine=innodb; +sync_slave_with_master; +connection master; +INSERT INTO t1 VALUES (1, 1, 1, 1); +BEGIN; +INSERT INTO t1 (`a`, `c`, `d`) VALUES (2, 2, 2) ON DUPLICATE KEY UPDATE c=99; + --connection master1 + INSERT INTO t1 (`a`, `c`, `d`) VALUES(3, 2, 3) ON DUPLICATE KEY UPDATE c=100; +--connection master +COMMIT; +SELECT * FROM t1; +--sync_slave_with_master +--echo #same data as master +SELECT * FROM t1; +connection master; +drop table t1; +--sync_slave_with_master + +--echo # Case 4: UNSAFE --connection master CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT, UNIQUE(b), c int) engine=innodb; diff --git a/mysql-test/suite/rpl/t/rpl_unsafe_statements.test b/mysql-test/suite/rpl/t/rpl_unsafe_statements.test index aa0bd076398..cbb4b54a220 100644 --- a/mysql-test/suite/rpl/t/rpl_unsafe_statements.test +++ b/mysql-test/suite/rpl/t/rpl_unsafe_statements.test @@ -24,7 +24,7 @@ --source include/have_innodb.inc --source include/have_binlog_format_mixed.inc --source include/master-slave.inc -call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); + # Case-1: BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS # Statement is unsafe because it invokes a trigger or a # stored function that inserts into an AUTO_INCREMENT column. diff --git a/sql/sql_class.cc b/sql/sql_class.cc index aa64e42e144..67373c99966 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -6285,47 +6285,84 @@ int THD::decide_logging_format(TABLE_LIST *tables) DBUG_RETURN(0); } -int THD::decide_logging_format_low(TABLE *table) + +/* + Reconsider logging format in case of INSERT...ON DUPLICATE KEY UPDATE + for tables with more than one unique keys in case of MIXED binlog format. + + Unsafe means that a master could execute the statement differently than + the slave. + This could can happen in the following cases: + - The unique check are done in different order on master or slave + (different engine or different key order). + - There is a conflict on another key than the first and before the + statement is committed, another connection commits a row that conflicts + on an earlier unique key. Example follows: + + Below a and b are unique keys, the table has a row (1,1,0) + connection 1: + INSERT INTO t1 set a=2,b=1,c=0 ON DUPLICATE KEY UPDATE c=1; + connection 2: + INSERT INTO t1 set a=2,b=2,c=0; + + If 2 commits after 1 has been executed but before 1 has committed + (and are thus put before the other in the binary log), one will + get different data on the slave: + (1,1,1),(2,2,1) instead of (1,1,1),(2,2,0) +*/ + +void THD::reconsider_logging_format_for_iodup(TABLE *table) { - /* - INSERT...ON DUPLICATE KEY UPDATE on a table with more than one unique keys - can be unsafe. - */ - if(wsrep_binlog_format() <= BINLOG_FORMAT_STMT && - !is_current_stmt_binlog_format_row() && - !lex->is_stmt_unsafe() && - lex->sql_command == SQLCOM_INSERT && - lex->duplicates == DUP_UPDATE) + DBUG_ENTER("reconsider_logging_format_for_iodup"); + enum_binlog_format bf= (enum_binlog_format) wsrep_binlog_format(); + + DBUG_ASSERT(lex->duplicates == DUP_UPDATE); + + if (bf <= BINLOG_FORMAT_STMT && + !is_current_stmt_binlog_format_row()) { + KEY *end= table->s->key_info + table->s->keys; uint unique_keys= 0; - uint keys= table->s->keys, i= 0; - Field *field; - for (KEY* keyinfo= table->s->key_info; - i < keys && unique_keys <= 1; i++, keyinfo++) - if (keyinfo->flags & HA_NOSAME && - !(keyinfo->key_part->field->flags & AUTO_INCREMENT_FLAG && - //User given auto inc can be unsafe - !keyinfo->key_part->field->val_int())) + + for (KEY *keyinfo= table->s->key_info; keyinfo < end ; keyinfo++) + { + if (keyinfo->flags & HA_NOSAME) { + /* + We assume that the following cases will guarantee that the + key is unique if a key part is not set: + - The key part is an autoincrement (autogenerated) + - The key part has a default value that is null and it not + a virtual field that will be calculated later. + */ for (uint j= 0; j < keyinfo->user_defined_key_parts; j++) { - field= keyinfo->key_part[j].field; - if(!bitmap_is_set(table->write_set,field->field_index)) - goto exit; + Field *field= keyinfo->key_part[j].field; + if (!bitmap_is_set(table->write_set, field->field_index)) + { + /* Check auto_increment */ + if (field == table->next_number_field) + goto exit; + if (field->is_real_null() && !field->default_value) + goto exit; + } } - unique_keys++; + if (unique_keys++) + break; exit:; } - + } if (unique_keys > 1) { - lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS); - binlog_unsafe_warning_flags|= lex->get_stmt_unsafe_flags(); + if (bf == BINLOG_FORMAT_STMT && !lex->is_stmt_unsafe()) + { + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS); + binlog_unsafe_warning_flags|= lex->get_stmt_unsafe_flags(); + } set_current_stmt_binlog_format_row_if_mixed(); - return 1; } } - return 0; + DBUG_VOID_RETURN; } /* diff --git a/sql/sql_class.h b/sql/sql_class.h index cceefb1793c..e3d9bc9ded0 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -4294,18 +4294,18 @@ public: mdl_context.release_transactional_locks(); } int decide_logging_format(TABLE_LIST *tables); - /* - In Some cases when decide_logging_format is called it does not have all - information to decide the logging format. So that cases we call decide_logging_format_2 - at later stages in execution. - One example would be binlog format for IODKU but column with unique key is not inserted. - We dont have inserted columns info when we call decide_logging_format so on later stage we call - decide_logging_format_low - @returns 0 if no format is changed - 1 if there is change in binlog format + /* + In Some cases when decide_logging_format is called it does not have + all information to decide the logging format. So that cases we call + decide_logging_format_2 at later stages in execution. + + One example would be binlog format for insert on duplicate key + (IODKU) but column with unique key is not inserted. We do not have + inserted columns info when we call decide_logging_format so on + later stage we call reconsider_logging_format_for_iodup() */ - int decide_logging_format_low(TABLE *table); + void reconsider_logging_format_for_iodup(TABLE *table); enum need_invoker { INVOKER_NONE=0, INVOKER_USER, INVOKER_ROLE}; void binlog_invoker(bool role) { m_binlog_invoker= role ? INVOKER_ROLE : INVOKER_USER; } diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 13dbbaed539..5137e7230d4 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -943,6 +943,11 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, goto values_loop_end; } } + if (duplic == DUP_UPDATE) + { + restore_record(table,s->default_values); // Get empty record + thd->reconsider_logging_format_for_iodup(table); + } do { DBUG_PRINT("info", ("iteration %llu", iteration)); @@ -1051,7 +1056,6 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, break; } - thd->decide_logging_format_low(table); #ifndef EMBEDDED_LIBRARY if (lock_type == TL_WRITE_DELAYED) {