From 60916a8b9e81b9e9de14fa737848419f45ff899a Mon Sep 17 00:00:00 2001 From: Monty Date: Sat, 28 May 2016 01:15:39 +0300 Subject: [PATCH] Simplify THD::decide_logging_format() Fixed some test for future when DELETE will not trigger row based replication --- .../suite/rpl/r/rpl_temp_table_mix_row.result | 10 +- .../suite/rpl/t/rpl_temp_table_mix_row.test | 3 +- sql/sql_class.cc | 163 +++++------------- 3 files changed, 58 insertions(+), 118 deletions(-) diff --git a/mysql-test/suite/rpl/r/rpl_temp_table_mix_row.result b/mysql-test/suite/rpl/r/rpl_temp_table_mix_row.result index e330576be54..585ebfd9bdb 100644 --- a/mysql-test/suite/rpl/r/rpl_temp_table_mix_row.result +++ b/mysql-test/suite/rpl/r/rpl_temp_table_mix_row.result @@ -31,7 +31,8 @@ CREATE TABLE t3 ( i1 INT NOT NULL AUTO_INCREMENT, PRIMARY KEY (i1) ); CREATE TRIGGER tr1 AFTER DELETE ON t2 FOR EACH ROW INSERT INTO t3 () VALUES (); CREATE TEMPORARY TABLE t1_tmp (i1 int); ALTER TABLE t1_tmp ADD COLUMN b INT; -DELETE FROM t2; +INSERT INTO t1 values(1); +INSERT INTO t2 (i1) select * from t1; CREATE TEMPORARY TABLE t2_tmp (a int); ALTER TABLE t1_tmp ADD COLUMN c INT; ### assertion: assert that there is one open temp table on slave @@ -66,6 +67,13 @@ slave-bin.000001 # Gtid # # GTID #-#-# slave-bin.000001 # Query # # use `test`; CREATE TEMPORARY TABLE t1_tmp (i1 int) slave-bin.000001 # Gtid # # GTID #-#-# slave-bin.000001 # Query # # use `test`; ALTER TABLE t1_tmp ADD COLUMN b INT +slave-bin.000001 # Gtid # # BEGIN GTID #-#-# +slave-bin.000001 # Query # # use `test`; INSERT INTO t1 values(1) +slave-bin.000001 # Xid # # COMMIT /* XID */ +slave-bin.000001 # Gtid # # BEGIN GTID #-#-# +slave-bin.000001 # Table_map # # table_id: # (test.t2) +slave-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F +slave-bin.000001 # Query # # COMMIT slave-bin.000001 # Gtid # # GTID #-#-# slave-bin.000001 # Query # # DROP TEMPORARY TABLE IF EXISTS `test`.`t1_tmp` /* generated by server */ slave-bin.000001 # Gtid # # GTID #-#-# diff --git a/mysql-test/suite/rpl/t/rpl_temp_table_mix_row.test b/mysql-test/suite/rpl/t/rpl_temp_table_mix_row.test index b221abf8a4a..fa611538aed 100644 --- a/mysql-test/suite/rpl/t/rpl_temp_table_mix_row.test +++ b/mysql-test/suite/rpl/t/rpl_temp_table_mix_row.test @@ -102,7 +102,8 @@ CREATE TEMPORARY TABLE t1_tmp (i1 int); ALTER TABLE t1_tmp ADD COLUMN b INT; # action: force switch to RBR -DELETE FROM t2; +INSERT INTO t1 values(1); +INSERT INTO t2 (i1) select * from t1; # assertion: assert that t2_tmp will not make into the binlog (RBR logging atm) CREATE TEMPORARY TABLE t2_tmp (a int); diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 0f9afb9c253..d016bfdae50 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -5528,94 +5528,6 @@ int xid_cache_iterate(THD *thd, my_hash_walk_action action, void *arg) &argument); } -/* - 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 @@ -5746,17 +5658,24 @@ int THD::decide_logging_format(TABLE_LIST *tables) If different types of engines are about to be updated. For example: Innodb and Falcon; Innodb and MyIsam. */ - my_bool multi_write_engine= FALSE; + bool multi_write_engine= FALSE; /* If different types of engines are about to be accessed and any of them is about to be updated. For example: Innodb and Falcon; Innodb and MyIsam. */ - my_bool multi_access_engine= FALSE; + bool multi_access_engine= FALSE; /* Identifies if a table is changed. */ - my_bool is_write= FALSE; + bool is_write= FALSE; // If any write tables + bool has_read_tables= FALSE; // If any read only tables + bool has_auto_increment_write_tables= FALSE; // Write with auto-increment + /* If a write table that doesn't have auto increment part first */ + bool has_write_table_auto_increment_not_first_in_pk= FALSE; + bool has_auto_increment_write_tables_not_first= FALSE; + bool found_first_not_own_table= FALSE; + /* A pointer to a previous table that was changed. */ @@ -5800,31 +5719,6 @@ int THD::decide_logging_format(TABLE_LIST *tables) } #endif - if (wsrep_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. @@ -5863,9 +5757,22 @@ int THD::decide_logging_format(TABLE_LIST *tables) continue; } } + if (table == lex->first_not_own_table()) + found_first_not_own_table= true; replicated_tables_count++; + if (table->lock_type <= TL_READ_NO_INSERT) + has_read_tables= true; + else if (table->table->found_next_number_field && + (table->lock_type >= TL_WRITE_ALLOW_WRITE)) + { + has_auto_increment_write_tables= true; + has_auto_increment_write_tables_not_first= found_first_not_own_table; + if (table->table->s->next_number_keypart != 0) + has_write_table_auto_increment_not_first_in_pk= true; + } + if (table->lock_type >= TL_WRITE_ALLOW_WRITE) { if (prev_write_table && prev_write_table->file->ht != @@ -5910,6 +5817,30 @@ int THD::decide_logging_format(TABLE_LIST *tables) prev_access_table= table->table; } + if (wsrep_binlog_format() != BINLOG_FORMAT_ROW) + { + /* + 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_auto_increment_write_tables && has_read_tables) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT); + + if (has_write_table_auto_increment_not_first_in_pk) + 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_auto_increment_write_tables_not_first) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS); + } + DBUG_PRINT("info", ("flags_write_all_set: 0x%llx", flags_write_all_set)); DBUG_PRINT("info", ("flags_write_some_set: 0x%llx", flags_write_some_set)); DBUG_PRINT("info", ("flags_access_some_set: 0x%llx", flags_access_some_set));