revision-id: 5619f0a24fc89f0b78d23a179fcf9203c1e9da8a (mariadb-10.1.39-91-g5619f0a24fc) parent(s): bdc961acc7ae9d5a3661ac425e936cd03dc5ccbb author: Sachin committer: Sachin timestamp: 2019-07-17 15:56:29 +0530 message: MDEV-17614 INSERT on dup key update is replication unsafe Make INSERT...ON DUPLICATE KEY UPDATE unsafe while using stmt or mixed format When there is more then one unique key. Although there is two exception. 1. Auto Increment key is not counted because Innodb will get gap lock for failed Insert and concurrent insert will get a next increment value. But if user supplies auto inc value it can be unsafe. 2. Count only unique keys for which insertion is performed. So this patch also addresses the bug id #72921 --- .../suite/rpl/r/rpl_known_bugs_detection.result | 20 ---- mysql-test/suite/rpl/r/rpl_mdev_17614.result | 98 +++++++++++++++++ .../suite/rpl/t/rpl_known_bugs_detection.test | 39 ------- mysql-test/suite/rpl/t/rpl_mdev_17614.test | 121 +++++++++++++++++++++ sql/sql_class.cc | 42 +++++++ sql/sql_class.h | 40 ++++--- sql/sql_insert.cc | 1 + 7 files changed, 288 insertions(+), 73 deletions(-) diff --git a/mysql-test/suite/rpl/r/rpl_known_bugs_detection.result b/mysql-test/suite/rpl/r/rpl_known_bugs_detection.result index ea738b710fd..adef091ea3e 100644 --- a/mysql-test/suite/rpl/r/rpl_known_bugs_detection.result +++ b/mysql-test/suite/rpl/r/rpl_known_bugs_detection.result @@ -1,26 +1,6 @@ call mtr.add_suppression("Unsafe statement written to the binary log using statement format"); 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 (a INT NOT NULL PRIMARY KEY AUTO_INCREMENT, b INT, -UNIQUE(b)); -INSERT INTO t1(b) VALUES(1),(1),(2) ON DUPLICATE KEY UPDATE t1.b=10; -SELECT * FROM t1; -a b -1 10 -2 2 -call mtr.add_suppression("Slave SQL.*suffer.*http:..bugs.mysql.com.bug.php.id=24432"); -include/wait_for_slave_sql_error.inc [errno=1105] -Last_SQL_Error = 'Error 'master may suffer from http://bugs.mysql.com/bug.php?id=24432 so slave stops; check error log on slave for more info' on query. Default database: 'test'. Query: 'INSERT INTO t1(b) VALUES(1),(1),(2) ON DUPLICATE KEY UPDATE t1.b=10'' -SELECT * FROM t1; -a b -stop slave; -include/wait_for_slave_to_stop.inc -reset slave; -reset master; -drop table t1; -start slave; -include/wait_for_slave_to_start.inc CREATE TABLE t1 ( id bigint(20) unsigned NOT NULL auto_increment, field_1 int(10) unsigned NOT NULL, diff --git a/mysql-test/suite/rpl/r/rpl_mdev_17614.result b/mysql-test/suite/rpl/r/rpl_mdev_17614.result new file mode 100644 index 00000000000..28de23e28c9 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_mdev_17614.result @@ -0,0 +1,98 @@ +include/master-slave.inc +[connection master] +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; +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 +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 +COMMIT; +SELECT * FROM t1; +a b c +1 1 2 +2 2 3 +include/wait_for_slave_sql_error.inc [errno=1062] +Last_SQL_Error = 'Error 'Duplicate entry '1' for key 'b'' on query. Default database: 'test'. Query: 'INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c)'' +#Different value from server +SELECT * FROM t1; +a b c +1 1 1 +2 2 3 +stop slave; +include/wait_for_slave_to_stop.inc +reset slave; +reset master; +drop table t1; +start slave; +include/wait_for_slave_to_start.inc +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT, +UNIQUE(b), c int) engine=innodb; +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); +INSERT INTO t1 VALUES(default, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); +COMMIT; +SELECT * FROM t1; +a b c +1 1 2 +3 2 3 +#same data as master +SELECT * FROM t1; +a b c +1 1 2 +3 2 3 +drop table t1; +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY, b INT, +UNIQUE(b), c int, d int ) engine=innodb; +INSERT INTO t1 VALUES (1, 1, 1, 1); +BEGIN; +INSERT INTO t1 VALUES (2, NULL, 2, 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 +INSERT INTO t1 VALUES(3, NULL, 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 +COMMIT; +SELECT * FROM t1; +a b c d +1 1 1 1 +2 NULL 2 2 +3 NULL 2 3 +#same data as master +SELECT * FROM t1; +a b c d +1 1 1 1 +2 NULL 2 2 +3 NULL 2 3 +drop table t1; +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT, +UNIQUE(b), c int) engine=innodb; +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); +INSERT INTO t1 VALUES(2, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); +COMMIT; +SELECT * FROM t1; +a b c +1 1 2 +2 2 3 +include/wait_for_slave_sql_error.inc [errno=1062] +Last_SQL_Error = 'Error 'Duplicate entry '1' for key 'b'' on query. Default database: 'test'. Query: 'INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c)'' +#Different value from server +SELECT * FROM t1; +a b c +1 1 1 +2 2 3 +stop slave; +include/wait_for_slave_to_stop.inc +reset slave; +reset master; +drop table t1; +start slave; +include/wait_for_slave_to_start.inc +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_known_bugs_detection.test b/mysql-test/suite/rpl/t/rpl_known_bugs_detection.test index ab263ece407..5ea056d5f14 100644 --- a/mysql-test/suite/rpl/t/rpl_known_bugs_detection.test +++ b/mysql-test/suite/rpl/t/rpl_known_bugs_detection.test @@ -14,45 +14,6 @@ source include/have_binlog_checksum_off.inc; source include/master-slave.inc; -call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT."); - -# -# This is to test that slave properly detects if -# master may suffer from: -# BUG#24432 "INSERT... ON DUPLICATE KEY UPDATE skips auto_increment values" -# (i.e. on master, INSERT ON DUPLICATE KEY UPDATE is used and manipulates -# an auto_increment column, and is binlogged statement-based). -# - -# testcase with INSERT VALUES -CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY AUTO_INCREMENT, b INT, -UNIQUE(b)); -sync_slave_with_master; -connection master; -INSERT INTO t1(b) VALUES(1),(1),(2) ON DUPLICATE KEY UPDATE t1.b=10; -SELECT * FROM t1; -connection slave; - -# show the error message -#1105 = ER_UNKNOWN_ERROR ---let $slave_sql_errno= 1105 ---let $show_slave_sql_error= 1 -call mtr.add_suppression("Slave SQL.*suffer.*http:..bugs.mysql.com.bug.php.id=24432"); ---source include/wait_for_slave_sql_error.inc -# show that it was not replicated -SELECT * FROM t1; - -# restart replication for the next testcase -stop slave; ---source include/wait_for_slave_to_stop.inc -reset slave; -connection master; -reset master; -drop table t1; -connection slave; -start slave; ---source include/wait_for_slave_to_start.inc - # testcase with INSERT SELECT connection master; CREATE TABLE t1 ( diff --git a/mysql-test/suite/rpl/t/rpl_mdev_17614.test b/mysql-test/suite/rpl/t/rpl_mdev_17614.test new file mode 100644 index 00000000000..9b86c8c15b5 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_mdev_17614.test @@ -0,0 +1,121 @@ +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 + +# Case 1 +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; +sync_slave_with_master; +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); + --connection master1 + INSERT INTO t1 VALUES(2, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); +--connection master +COMMIT; +SELECT * FROM t1; +--connection slave +# show the error message +--let $slave_sql_errno= 1062 +--let $show_slave_sql_error= 1 +--source include/wait_for_slave_sql_error.inc +--echo #Different value from server +SELECT * FROM t1; + +# restart replication for the next testcase +stop slave; +--source include/wait_for_slave_to_stop.inc +reset slave; +connection master; +reset master; +drop table t1; +connection slave; +start slave; +--source include/wait_for_slave_to_start.inc +# Case 2 +--connection master +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT, +UNIQUE(b), c int) engine=innodb; +sync_slave_with_master; +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); + --connection master1 + INSERT INTO t1 VALUES(default, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), 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 + +# Case 3 +--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 b=VALUES(b), c=VALUES(c); + --connection master1 + INSERT INTO t1 VALUES(3, NULL, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), 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 + +# Case 4 +--connection master +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY auto_increment, b INT, +UNIQUE(b), c int) engine=innodb; +sync_slave_with_master; +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); + --connection master1 + INSERT INTO t1 VALUES(2, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); +--connection master +COMMIT; +SELECT * FROM t1; +--connection slave +# show the error message +--let $slave_sql_errno= 1062 +--let $show_slave_sql_error= 1 +--source include/wait_for_slave_sql_error.inc +--echo #Different value from server +SELECT * FROM t1; + +# restart replication for the next testcase +stop slave; +--source include/wait_for_slave_to_stop.inc +reset slave; +connection master; +reset master; +drop table t1; +connection slave; +start slave; +--source include/wait_for_slave_to_start.inc + +--source include/rpl_end.inc diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 6bcff6d1fca..b9c499e9db7 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -6144,6 +6144,48 @@ int THD::decide_logging_format(TABLE_LIST *tables) DBUG_RETURN(0); } +int THD::decide_logging_format_2(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) + { + 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 (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; + } + unique_keys++; +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(); + set_current_stmt_binlog_format_row_if_mixed(); + return 1; + } + } + return 0; +} /* Implementation of interface to write rows to the binary log through the diff --git a/sql/sql_class.h b/sql/sql_class.h index e3bc572ed0c..260cf8039ea 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2208,6 +2208,20 @@ class THD :public Statement, /* container for handler's private per-connection data */ Ha_data ha_data[MAX_HA]; + /** + Bit field for the state of binlog warnings. + + The first Lex::BINLOG_STMT_UNSAFE_COUNT bits list all types of + unsafeness that the current statement has. + + This must be a member of THD and not of LEX, because warnings are + detected and issued in different places (@c + decide_logging_format() and @c binlog_query(), respectively). + Between these calls, the THD->lex object may change; e.g., if a + stored routine is invoked. Only THD persists between the calls. + */ + uint32 binlog_unsafe_warning_flags; + #ifndef MYSQL_CLIENT binlog_cache_mngr * binlog_setup_trx_data(); @@ -2317,20 +2331,6 @@ class THD :public Statement, */ enum_binlog_format current_stmt_binlog_format; - /** - Bit field for the state of binlog warnings. - - The first Lex::BINLOG_STMT_UNSAFE_COUNT bits list all types of - unsafeness that the current statement has. - - This must be a member of THD and not of LEX, because warnings are - detected and issued in different places (@c - decide_logging_format() and @c binlog_query(), respectively). - Between these calls, the THD->lex object may change; e.g., if a - stored routine is invoked. Only THD persists between the calls. - */ - uint32 binlog_unsafe_warning_flags; - /* Number of outstanding table maps, i.e., table maps in the transaction cache. @@ -3939,6 +3939,18 @@ class THD :public Statement, } void leave_locked_tables_mode(); 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_2 + + @returns 0 if no format is changed + 1 if there is change in binlog format + */ + int decide_logging_format_2(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 da76f10596d..30df68d8c6e 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1025,6 +1025,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, error= 1; break; } + thd->decide_logging_format_2(table); #ifndef EMBEDDED_LIBRARY if (lock_type == TL_WRITE_DELAYED) {