diff options
| -rw-r--r-- | mysql-test/suite/rpl/r/rpl_unsafe_statements.result | 53 | ||||
| -rw-r--r-- | mysql-test/suite/rpl/t/rpl_unsafe_statements.test | 175 | ||||
| -rw-r--r-- | sql/sql_base.cc | 154 | ||||
| -rw-r--r-- | sql/sql_class.cc | 131 | ||||
| -rw-r--r-- | sql/sql_lex.h | 7 |
5 files changed, 365 insertions, 155 deletions
diff --git a/mysql-test/suite/rpl/r/rpl_unsafe_statements.result b/mysql-test/suite/rpl/r/rpl_unsafe_statements.result new file mode 100644 index 00000000000..2efb3eba2b1 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_unsafe_statements.result @@ -0,0 +1,53 @@ +include/master-slave.inc +[connection master] +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 +FOR EACH ROW +INSERT INTO t2(i) VALUES(new.i); +START TRANSACTION; +INSERT INTO t2(i) VALUES (1); +ROLLBACK; +INSERT INTO t1(i) VALUES(2); +START TRANSACTION; +LOCK TABLES t1 WRITE, t2 WRITE; +INSERT INTO t1(i) VALUES(3); +UNLOCK TABLES; +COMMIT; +include/diff_tables.inc [master:t1, slave:t1] +include/diff_tables.inc [master:t2, slave:t2] +DROP TABLE t1,t2; +CREATE TABLE t1(i INT) ENGINE=INNODB; +CREATE TABLE t2(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB; +INSERT INTO t1 values (1), (2), (3); +START TRANSACTION; +INSERT INTO t2(i) VALUES (1); +ROLLBACK; +INSERT INTO t2(i) SELECT i FROM t1; +START TRANSACTION; +LOCK TABLES t2 WRITE, t1 READ; +INSERT INTO t2(i) SELECT i FROM t1; +UNLOCK TABLES; +COMMIT; +include/diff_tables.inc [master:t1, slave:t1] +include/diff_tables.inc [master:t2, slave:t2] +DROP TABLE t1,t2; +CREATE TABLE t1(i int, id INT AUTO_INCREMENT, PRIMARY KEY (i, id)) ENGINE=MYISAM; +INSERT INTO t1 (i) values (1); +START TRANSACTION; +LOCK TABLES t1 WRITE; +INSERT INTO t1 (i) values (2); +UNLOCK TABLES; +COMMIT; +include/diff_tables.inc [master:t1, slave:t1] +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; +START TRANSACTION; +LOCK TABLES t1 WRITE; +INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1; +UNLOCK TABLES; +COMMIT; +include/diff_tables.inc [master:t1, slave:t1] +DROP TABLE t1; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_unsafe_statements.test b/mysql-test/suite/rpl/t/rpl_unsafe_statements.test new file mode 100644 index 00000000000..def8e8f50ec --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_unsafe_statements.test @@ -0,0 +1,175 @@ +################################################################################ +# Bug#17047208 REPLICATION DIFFERENCE FOR MULTIPLE TRIGGERS +# Problem: If DML invokes a trigger or a stored function that inserts into an +# AUTO_INCREMENT column, that DML has to be marked as 'unsafe' statement. If the +# tables are locked in the transaction prior to DML statement (using LOCK +# TABLES), then the DML statement is not marked as 'unsafe' statement. + +# Steps to reproduce the reported test case (BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS) +# Case-1: +# > Create a trigger on a table and do a insert in the trigger that updates +# auto increment column +# > A DML that executes the trigger in step.1 and check that DML is marked +# as unsafe and DML is written into binlog using row format (in MBR) +# > Execute the step 2 by locking the required tables prior to DML and check +# that DML is marked as unsafe and DML is written into binlog using row +# format (in MBR) +# +# This test script also adds test cases to cover few other unsafe statements. +# Case-2: BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT +# Case-3: BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST +# Case-4: BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS +################################################################################ + +--source include/have_binlog_format_mixed.inc +--source include/master-slave.inc + +# 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. + +# Step-1.1: Create two tables, one with AUTO_INCREMENT column. +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; + +# Step-1.2: Create a trigger that inserts into an AUTO_INCREMENT column. +CREATE TRIGGER trig1 AFTER INSERT ON t1 +FOR EACH ROW + INSERT INTO t2(i) VALUES(new.i); + +# Step-1.3: Create some gap in auto increment value on master's t2 table +# but not on slave (by doing rollback). Just in case if the unsafe statements +# are written in statement format, diff tables will fail. +START TRANSACTION; +INSERT INTO t2(i) VALUES (1); +ROLLBACK; + +# Step-1.4: Insert a tuple into table t1 that triggers trig1 which inserts +# into an AUTO_INCREMENT column. +INSERT INTO t1(i) VALUES(2); + +# Step-1.5: Repeat step 1.4 but using 'LOCK TABLES' logic. +START TRANSACTION; +LOCK TABLES t1 WRITE, t2 WRITE; +INSERT INTO t1(i) VALUES(3); +UNLOCK TABLES; +COMMIT; + +# Step-1.6: Sync slave with master +--sync_slave_with_master + +# Step-1.7: Diff master-slave tables to make sure everything is in sync. +--let $diff_tables=master:t1, slave:t1 +--source include/diff_tables.inc + +--let $diff_tables=master:t2, slave:t2 +--source include/diff_tables.inc + +# Step-1.8: Cleanup +--connection master +DROP TABLE t1,t2; + +# Case-2: BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT +# Statements writing to a table with an auto-increment column after selecting +# from another table are unsafe because the order in which rows are retrieved +# determines what (if any) rows will be written. This order cannot be +# predicted and may differ on master and the slave. + +# Step-2.1: Create two tables, one with AUTO_INCREMENT column. +CREATE TABLE t1(i INT) ENGINE=INNODB; +CREATE TABLE t2(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB; + +# Step-2.2: Create some tuples in table t1. +INSERT INTO t1 values (1), (2), (3); + +# Step-2.3: Create some gap in auto increment value on master's t2 table +# but not on slave (by doing rollback). Just in case if the unsafe statements +# are written in statement format, diff tables will fail. +START TRANSACTION; +INSERT INTO t2(i) VALUES (1); +ROLLBACK; + +# Step-2.4: Insert into t2 (table with an auto-increment) by selecting tuples +# from table t1. +INSERT INTO t2(i) SELECT i FROM t1; + +# Step-2.5: Repeat step 2.4 but now with 'LOCK TABLES' logic. +START TRANSACTION; +LOCK TABLES t2 WRITE, t1 READ; +INSERT INTO t2(i) SELECT i FROM t1; +UNLOCK TABLES; +COMMIT; + +# Step-2.6: Sync slave with master +--sync_slave_with_master + +# Step-2.7: Diff master-slave tables to make sure everything is in sync. +--let $diff_tables=master:t1, slave:t1 +--source include/diff_tables.inc + +--let $diff_tables=master:t2, slave:t2 +--source include/diff_tables.inc + +# Step-2.8: Cleanup +--connection master +DROP TABLE t1,t2; + +# Case-3: BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST +# INSERT into autoincrement field which is not the first part in the +# composed primary key is unsafe +# +# Step-3.1: Create a table with auto increment column and a composed primary key +# (second column is auto increment column). Such a definition is allowed only +# with 'myisam' engine. +CREATE TABLE t1(i int, id INT AUTO_INCREMENT, PRIMARY KEY (i, id)) ENGINE=MYISAM; + +# Step-3.2: Inserting into such a table is unsafe. +INSERT INTO t1 (i) values (1); + +# Step-3.3: Repeat step 3.2, now with 'LOCK TABLES' logic. +START TRANSACTION; +LOCK TABLES t1 WRITE; +INSERT INTO t1 (i) values (2); +UNLOCK TABLES; +COMMIT; + +# Step-3.4: Sync slave with master +--sync_slave_with_master + +# Step-3.5: Diff master-slave tables to make sure everything is in sync. +--let $diff_tables=master:t1, slave:t1 +--source include/diff_tables.inc + +# Step-3.6: Cleanup +--connection master +DROP TABLE t1; + +# Case-4: BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS +# INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY +# is unsafe Statement + +# Step-4.1: Create a table with two unique keys +CREATE TABLE t1(i INT, j INT, UNIQUE KEY(i), UNIQUE KEY(j)) ENGINE=INNODB; + +# Step-4.2: Inserting into such a table is unsafe. +INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1; + +# Step-4.3: Repeat step 3.2, now with 'LOCK TABLES' logic. +START TRANSACTION; +LOCK TABLES t1 WRITE; +INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1; +UNLOCK TABLES; +COMMIT; + +# Step-4.4: Sync slave with master +--sync_slave_with_master + +# Step-4.5: Diff master-slave tables to make sure everything is in sync. +--let $diff_tables=master:t1, slave:t1 +--source include/diff_tables.inc + +# Step-4.6: Cleanup +--connection master +DROP TABLE t1; + +--source include/rpl_end.inc diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 172987cc1c6..f559974c86b 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -212,11 +212,6 @@ static bool check_and_update_table_version(THD *thd, TABLE_LIST *tables, static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, TABLE *entry); static bool auto_repair_table(THD *thd, TABLE_LIST *table_list); static void free_cache_entry(TABLE *entry); -static bool -has_write_table_with_auto_increment(TABLE_LIST *tables); -static bool -has_write_table_with_auto_increment_and_select(TABLE_LIST *tables); -static bool has_write_table_auto_increment_not_first_in_pk(TABLE_LIST *tables); uint cached_open_tables(void) { @@ -5744,63 +5739,6 @@ bool lock_tables(THD *thd, TABLE_LIST *tables, uint count, *(ptr++)= table->table; } - /* - DML statements that modify a table with an auto_increment column based on - rows selected from a table are unsafe as the order in which the rows are - fetched fron the select tables cannot be determined and may differ on - master and slave. - */ - if (thd->variables.binlog_format != BINLOG_FORMAT_ROW && tables && - has_write_table_with_auto_increment_and_select(tables)) - thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT); - /* Todo: merge all has_write_table_auto_inc with decide_logging_format */ - if (thd->variables.binlog_format != BINLOG_FORMAT_ROW && tables) - { - if (has_write_table_auto_increment_not_first_in_pk(tables)) - thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST); - } - - /* - INSERT...ON DUPLICATE KEY UPDATE on a table with more than one unique keys - can be unsafe. - */ - uint unique_keys= 0; - for (TABLE_LIST *query_table= tables; query_table && unique_keys <= 1; - query_table= query_table->next_global) - if(query_table->table) - { - uint keys= query_table->table->s->keys, i= 0; - unique_keys= 0; - for (KEY* keyinfo= query_table->table->s->key_info; - i < keys && unique_keys <= 1; i++, keyinfo++) - { - if (keyinfo->flags & HA_NOSAME) - unique_keys++; - } - if (!query_table->placeholder() && - query_table->lock_type >= TL_WRITE_ALLOW_WRITE && - unique_keys > 1 && thd->lex->sql_command == SQLCOM_INSERT && - /* Duplicate key update is not supported by INSERT DELAYED */ - thd->command != COM_DELAYED_INSERT && - thd->lex->duplicates == DUP_UPDATE) - thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS); - } - - /* We have to emulate LOCK TABLES if we are statement needs prelocking. */ - if (thd->lex->requires_prelocking()) - { - - /* - A query that modifies autoinc column in sub-statement can make the - master and slave inconsistent. - We can solve these problems in mixed mode by switching to binlogging - if at least one updated table is used by sub-statement - */ - if (thd->variables.binlog_format != BINLOG_FORMAT_ROW && tables && - has_write_table_with_auto_increment(thd->lex->first_not_own_table())) - thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS); - } - DEBUG_SYNC(thd, "before_lock_tables_takes_lock"); if (! (thd->lock= mysql_lock_tables(thd, start, (uint) (ptr - start), @@ -9170,98 +9108,6 @@ bool is_equal(const LEX_STRING *a, const LEX_STRING *b) return a->length == b->length && !strncmp(a->str, b->str, a->length); } - -/* - Tells if two (or more) tables have auto_increment columns and we want to - lock those tables with a write lock. - - SYNOPSIS - has_two_write_locked_tables_with_auto_increment - tables Table list - - NOTES: - Call this function only when you have established the list of all tables - which you'll want to update (including stored functions, triggers, views - inside your statement). -*/ - -static bool -has_write_table_with_auto_increment(TABLE_LIST *tables) -{ - for (TABLE_LIST *table= tables; table; table= table->next_global) - { - /* we must do preliminary checks as table->table may be NULL */ - if (!table->placeholder() && - table->table->found_next_number_field && - (table->lock_type >= TL_WRITE_ALLOW_WRITE)) - return 1; - } - - return 0; -} - -/* - checks if we have select tables in the table list and write tables - with auto-increment column. - - SYNOPSIS - has_two_write_locked_tables_with_auto_increment_and_select - tables Table list - - RETURN VALUES - - -true if the table list has atleast one table with auto-increment column - - - and atleast one table to select from. - -false otherwise -*/ - -static bool -has_write_table_with_auto_increment_and_select(TABLE_LIST *tables) -{ - bool has_select= false; - bool has_auto_increment_tables = has_write_table_with_auto_increment(tables); - for(TABLE_LIST *table= tables; table; table= table->next_global) - { - if (!table->placeholder() && - (table->lock_type <= TL_READ_NO_INSERT)) - { - has_select= true; - break; - } - } - return(has_select && has_auto_increment_tables); -} - -/* - Tells if there is a table whose auto_increment column is a part - of a compound primary key while is not the first column in - the table definition. - - @param tables Table list - - @return true if the table exists, fais if does not. -*/ - -static bool -has_write_table_auto_increment_not_first_in_pk(TABLE_LIST *tables) -{ - for (TABLE_LIST *table= tables; table; table= table->next_global) - { - /* we must do preliminary checks as table->table may be NULL */ - if (!table->placeholder() && - table->table->found_next_number_field && - (table->lock_type >= TL_WRITE_ALLOW_WRITE) - && table->table->s->next_number_keypart != 0) - return 1; - } - - return 0; -} - - - /* Open and lock system tables for read. diff --git a/sql/sql_class.cc b/sql/sql_class.cc index c3d2a092fd6..05bb38d8358 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -4142,6 +4142,94 @@ void xid_cache_delete(XID_STATE *xid_state) mysql_mutex_unlock(&LOCK_xid_cache); } +/* + Tells if two (or more) tables have auto_increment columns and we want to + lock those tables with a write lock. + + SYNOPSIS + has_two_write_locked_tables_with_auto_increment + tables Table list + + NOTES: + Call this function only when you have established the list of all tables + which you'll want to update (including stored functions, triggers, views + inside your statement). +*/ + +static bool +has_write_table_with_auto_increment(TABLE_LIST *tables) +{ + for (TABLE_LIST *table= tables; table; table= table->next_global) + { + /* we must do preliminary checks as table->table may be NULL */ + if (!table->placeholder() && + table->table->found_next_number_field && + (table->lock_type >= TL_WRITE_ALLOW_WRITE)) + return 1; + } + + return 0; +} + +/* + checks if we have select tables in the table list and write tables + with auto-increment column. + + SYNOPSIS + has_two_write_locked_tables_with_auto_increment_and_select + tables Table list + + RETURN VALUES + + -true if the table list has atleast one table with auto-increment column + + + and atleast one table to select from. + -false otherwise +*/ + +static bool +has_write_table_with_auto_increment_and_select(TABLE_LIST *tables) +{ + bool has_select= false; + bool has_auto_increment_tables = has_write_table_with_auto_increment(tables); + for(TABLE_LIST *table= tables; table; table= table->next_global) + { + if (!table->placeholder() && + (table->lock_type <= TL_READ_NO_INSERT)) + { + has_select= true; + break; + } + } + return(has_select && has_auto_increment_tables); +} + +/* + Tells if there is a table whose auto_increment column is a part + of a compound primary key while is not the first column in + the table definition. + + @param tables Table list + + @return true if the table exists, fais if does not. +*/ + +static bool +has_write_table_auto_increment_not_first_in_pk(TABLE_LIST *tables) +{ + for (TABLE_LIST *table= tables; table; table= table->next_global) + { + /* we must do preliminary checks as table->table may be NULL */ + if (!table->placeholder() && + table->table->found_next_number_field && + (table->lock_type >= TL_WRITE_ALLOW_WRITE) + && table->table->s->next_number_keypart != 0) + return 1; + } + + return 0; +} /** Decide on logging format to use for the statement and issue errors @@ -4305,6 +4393,31 @@ int THD::decide_logging_format(TABLE_LIST *tables) } #endif + if (variables.binlog_format != BINLOG_FORMAT_ROW && tables) + { + /* + DML statements that modify a table with an auto_increment column based on + rows selected from a table are unsafe as the order in which the rows are + fetched fron the select tables cannot be determined and may differ on + master and slave. + */ + if (has_write_table_with_auto_increment_and_select(tables)) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT); + + if (has_write_table_auto_increment_not_first_in_pk(tables)) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST); + + /* + A query that modifies autoinc column in sub-statement can make the + master and slave inconsistent. + We can solve these problems in mixed mode by switching to binlogging + if at least one updated table is used by sub-statement + */ + if (lex->requires_prelocking() && + has_write_table_with_auto_increment(lex->first_not_own_table())) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS); + } + /* Get the capabilities vector for all involved storage engines and mask out the flags for the binary log. @@ -4343,6 +4456,24 @@ int THD::decide_logging_format(TABLE_LIST *tables) prev_write_table= table->table; + /* + INSERT...ON DUPLICATE KEY UPDATE on a table with more than one unique keys + can be unsafe. Check for it if the flag is already not marked for the + given statement. + */ + if (!lex->is_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS) && + lex->sql_command == SQLCOM_INSERT && lex->duplicates == DUP_UPDATE) + { + uint keys= table->table->s->keys, i= 0, unique_keys= 0; + for (KEY* keyinfo= table->table->s->key_info; + i < keys && unique_keys <= 1; i++, keyinfo++) + { + if (keyinfo->flags & HA_NOSAME) + unique_keys++; + } + if (unique_keys > 1 ) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS); + } } flags_access_some_set |= flags; diff --git a/sql/sql_lex.h b/sql/sql_lex.h index c5886b3c142..867997feb39 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2000, 2014, Oracle and/or its affiliates. All rights reserved. +/* Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved. 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 @@ -1362,6 +1362,11 @@ public: return get_stmt_unsafe_flags() != 0; } + inline bool is_stmt_unsafe(enum_binlog_stmt_unsafe unsafe) + { + return binlog_stmt_flags & (1 << unsafe); + } + /** Flag the current (top-level) statement as unsafe. The flag will be reset after the statement has finished. |