Commit a5dc12ee authored by Andrei's avatar Andrei

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 91ab42a8 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.
parent 141ab971
--- 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;
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
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;
......
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;
......
--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
......@@ -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;
......
......@@ -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.
......
......@@ -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;
}
/*
......
......@@ -4294,18 +4294,18 @@ class THD :public Statement,
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
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.
@returns 0 if no format is changed
1 if there is change in binlog format
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; }
......
......@@ -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)
{
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment