From 7194aec8ba29371be796bd4b42773fd692a1123c Mon Sep 17 00:00:00 2001 From: Alfranio Correia Date: Mon, 22 Feb 2010 03:25:33 +0000 Subject: [PATCH] BUG#49019 Mixing self-logging eng. and regular eng. does not switch to row in mixed mode Reading from a self-logging engine and updating a transactional engine such as Innodb generates changes that are written to the binary log in the statement format and may make slaves diverge. In the mixed mode, such changes should be written to the binary log in the row format. Note that the issue does not happen if we mix a self-logging engine and MyIsam as this case is caught by checking the mixture of non-transactional and transactional engines. So, we classify a mixed statement where one reads from NDB and writes into another engine as unsafe: if (multi_engine && flags_some_set & HA_HAS_OWN_BINLOGGING) lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_MULTIPLE_ENGINES_AND_SELF_LOGGING_ENGINE); --- .../rpl_ndb_mixed_engines_transactions.result | 29 +++++++++ .../t/rpl_ndb_mixed_engines_transactions.test | 23 +++++++ sql/share/errmsg-utf8.txt | 3 + sql/sql_class.cc | 63 +++++++++++-------- sql/sql_lex.cc | 3 +- sql/sql_lex.h | 6 ++ 6 files changed, 100 insertions(+), 27 deletions(-) diff --git a/mysql-test/suite/rpl_ndb/r/rpl_ndb_mixed_engines_transactions.result b/mysql-test/suite/rpl_ndb/r/rpl_ndb_mixed_engines_transactions.result index da60b0bbbbf..7caa88a16a1 100644 --- a/mysql-test/suite/rpl_ndb/r/rpl_ndb_mixed_engines_transactions.result +++ b/mysql-test/suite/rpl_ndb/r/rpl_ndb_mixed_engines_transactions.result @@ -341,6 +341,25 @@ ROLLBACK; Warnings: Warning 1196 Some non-transactional changed tables couldn't be rolled back SET AUTOCOMMIT = 1; +---- Mixed statements Innodb ---- +BEGIN; +INSERT INTO tndb VALUES (147); +INSERT INTO tinnodb SELECT * FROM tndb ORDER BY a DESC LIMIT 1; +COMMIT; +INSERT INTO tndb VALUES (148); +BEGIN; +INSERT INTO tinnodb SELECT * FROM tndb ORDER BY a DESC LIMIT 1; +INSERT INTO tndb VALUES (149); +COMMIT; +BEGIN; +INSERT INTO tndb VALUES (150); +INSERT INTO tmyisam SELECT * FROM tndb ORDER BY a DESC LIMIT 1; +COMMIT; +INSERT INTO tndb VALUES (151); +BEGIN; +INSERT INTO tmyisam SELECT * FROM tndb ORDER BY a DESC LIMIT 1; +INSERT INTO tndb VALUES (152); +COMMIT; ==== Verify the result ==== SELECT * FROM tmyisam ORDER BY a; a @@ -393,6 +412,8 @@ a 140 142 146 +150 +151 SELECT * FROM tinnodb ORDER BY a; a 1 @@ -420,6 +441,8 @@ a 120 125 127 +147 +148 SELECT * FROM tndb ORDER BY a; a 2 @@ -447,6 +470,12 @@ a 121 123 126 +147 +148 +149 +150 +151 +152 [on slave] Comparing tables master:test.tmyisam and slave:test.tmyisam Comparing tables master:test.tinnodb and slave:test.tinnodb diff --git a/mysql-test/suite/rpl_ndb/t/rpl_ndb_mixed_engines_transactions.test b/mysql-test/suite/rpl_ndb/t/rpl_ndb_mixed_engines_transactions.test index 13c2fbce2b4..d93c5328000 100644 --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_mixed_engines_transactions.test +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_mixed_engines_transactions.test @@ -418,6 +418,29 @@ ROLLBACK; SET AUTOCOMMIT = 1; +--echo ---- Mixed statements Innodb ---- + +BEGIN; +INSERT INTO tndb VALUES (147); +INSERT INTO tinnodb SELECT * FROM tndb ORDER BY a DESC LIMIT 1; +COMMIT; + +INSERT INTO tndb VALUES (148); +BEGIN; +INSERT INTO tinnodb SELECT * FROM tndb ORDER BY a DESC LIMIT 1; +INSERT INTO tndb VALUES (149); +COMMIT; + +BEGIN; +INSERT INTO tndb VALUES (150); +INSERT INTO tmyisam SELECT * FROM tndb ORDER BY a DESC LIMIT 1; +COMMIT; + +INSERT INTO tndb VALUES (151); +BEGIN; +INSERT INTO tmyisam SELECT * FROM tndb ORDER BY a DESC LIMIT 1; +INSERT INTO tndb VALUES (152); +COMMIT; --echo ==== Verify the result ==== diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index 7ea8c75e43e..30d12e89ca0 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -6321,3 +6321,6 @@ ER_SPATIAL_MUST_HAVE_GEOM_COL 42000 ER_TOO_LONG_INDEX_COMMENT eng "Comment for index '%-.64s' is too long (max = %lu)" + +ER_BINLOG_UNSAFE_MULTIPLE_ENGINES_AND_SELF_LOGGING_ENGINE + eng "Mixing self-logging and non-self-logging engines in a statement is unsafe." diff --git a/sql/sql_class.cc b/sql/sql_class.cc index b7ded3b632f..436b9a587b0 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -3587,12 +3587,14 @@ int THD::decide_logging_format(TABLE_LIST *tables) capabilities, and one with the intersection of all the engine capabilities. */ + handler::Table_flags flags_write_some_set= 0; handler::Table_flags flags_some_set= 0; - handler::Table_flags flags_all_set= + handler::Table_flags flags_write_all_set= HA_BINLOG_ROW_CAPABLE | HA_BINLOG_STMT_CAPABLE; + my_bool multi_write_engine= FALSE; my_bool multi_engine= FALSE; - my_bool mixed_engine= FALSE; + my_bool trans_non_trans_multi_engine= FALSE; my_bool all_trans_engines= TRUE; TABLE* prev_write_table= NULL; TABLE* prev_access_table= NULL; @@ -3619,26 +3621,42 @@ int THD::decide_logging_format(TABLE_LIST *tables) continue; if (table->table->s->table_category == TABLE_CATEGORY_PERFORMANCE) lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_SYSTEM_TABLE); + handler::Table_flags const flags= table->table->file->ha_table_flags(); + DBUG_PRINT("info", ("table: %s; ha_table_flags: 0x%llx", + table->table_name, flags)); if (table->lock_type >= TL_WRITE_ALLOW_WRITE) { - handler::Table_flags const flags= table->table->file->ha_table_flags(); - DBUG_PRINT("info", ("table: %s; ha_table_flags: 0x%llx", - table->table_name, flags)); if (prev_write_table && prev_write_table->file->ht != table->table->file->ht) - multi_engine= TRUE; + multi_write_engine= TRUE; all_trans_engines= all_trans_engines && table->table->file->has_transactions(); prev_write_table= table->table; - flags_all_set &= flags; - flags_some_set |= flags; + flags_write_all_set &= flags; + flags_write_some_set |= flags; } + flags_some_set |= flags; if (prev_access_table && prev_access_table->file->ht != table->table->file->ht) - mixed_engine= mixed_engine || (prev_access_table->file->has_transactions() != - table->table->file->has_transactions()); + { + multi_engine= TRUE; + trans_non_trans_multi_engine= trans_non_trans_multi_engine || + (prev_access_table->file->has_transactions() != + table->table->file->has_transactions()); + } prev_access_table= table->table; } + 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_some_set: 0x%llx", flags_some_set)); + DBUG_PRINT("info", ("multi_write_engine: %d", multi_write_engine)); + DBUG_PRINT("info", ("multi_engine: %d", multi_engine)); + DBUG_PRINT("info", ("trans_non_trans_multi_engine: %d", + trans_non_trans_multi_engine)); + + int error= 0; + int unsafe_flags; + /* Set the statement as unsafe if: @@ -3708,32 +3726,25 @@ int THD::decide_logging_format(TABLE_LIST *tables) isolation level but if we have pure repeatable read or serializable the lock history on the slave will be different from the master. */ - if (mixed_engine || + if (trans_non_trans_multi_engine || (trans_has_updated_trans_table(this) && !all_trans_engines)) lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_NONTRANS_AFTER_TRANS); - DBUG_PRINT("info", ("flags_all_set: 0x%llx", flags_all_set)); - DBUG_PRINT("info", ("flags_some_set: 0x%llx", flags_some_set)); - DBUG_PRINT("info", ("multi_engine: %d", multi_engine)); - - int error= 0; - int unsafe_flags; - /* If more than one engine is involved in the statement and at least one is doing it's own logging (is *self-logging*), the statement cannot be logged atomically, so we generate an error rather than allowing the binlog to become corrupt. */ - if (multi_engine && - (flags_some_set & HA_HAS_OWN_BINLOGGING)) - { + if (multi_write_engine && + (flags_write_some_set & HA_HAS_OWN_BINLOGGING)) my_error((error= ER_BINLOG_MULTIPLE_ENGINES_AND_SELF_LOGGING_ENGINE), MYF(0)); - } + else if (multi_engine && flags_some_set & HA_HAS_OWN_BINLOGGING) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_MULTIPLE_ENGINES_AND_SELF_LOGGING_ENGINE); /* both statement-only and row-only engines involved */ - if ((flags_all_set & (HA_BINLOG_STMT_CAPABLE | HA_BINLOG_ROW_CAPABLE)) == 0) + if ((flags_write_all_set & (HA_BINLOG_STMT_CAPABLE | HA_BINLOG_ROW_CAPABLE)) == 0) { /* 1. Error: Binary logging impossible since both row-incapable @@ -3742,7 +3753,7 @@ int THD::decide_logging_format(TABLE_LIST *tables) my_error((error= ER_BINLOG_ROW_ENGINE_AND_STMT_ENGINE), MYF(0)); } /* statement-only engines involved */ - else if ((flags_all_set & HA_BINLOG_ROW_CAPABLE) == 0) + else if ((flags_write_all_set & HA_BINLOG_ROW_CAPABLE) == 0) { if (lex->is_stmt_row_injection()) { @@ -3790,7 +3801,7 @@ int THD::decide_logging_format(TABLE_LIST *tables) */ my_error((error= ER_BINLOG_ROW_INJECTION_AND_STMT_MODE), MYF(0)); } - else if ((flags_all_set & HA_BINLOG_STMT_CAPABLE) == 0) + else if ((flags_write_all_set & HA_BINLOG_STMT_CAPABLE) == 0) { /* 5. Error: Cannot modify table that uses a storage engine @@ -3818,7 +3829,7 @@ int THD::decide_logging_format(TABLE_LIST *tables) else { if (lex->is_stmt_unsafe() || lex->is_stmt_row_injection() - || (flags_all_set & HA_BINLOG_STMT_CAPABLE) == 0) + || (flags_write_all_set & HA_BINLOG_STMT_CAPABLE) == 0) { /* log in row format! */ set_current_stmt_binlog_format_row_if_mixed(); diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 6da734592dc..65dbe48aaaf 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -52,7 +52,8 @@ Query_tables_list::binlog_stmt_unsafe_errcode[BINLOG_STMT_UNSAFE_COUNT] = ER_BINLOG_UNSAFE_UDF, ER_BINLOG_UNSAFE_SYSTEM_VARIABLE, ER_BINLOG_UNSAFE_SYSTEM_FUNCTION, - ER_BINLOG_UNSAFE_NONTRANS_AFTER_TRANS + ER_BINLOG_UNSAFE_NONTRANS_AFTER_TRANS, + ER_BINLOG_UNSAFE_MULTIPLE_ENGINES_AND_SELF_LOGGING_ENGINE }; diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 7ec87806ea5..45a86464b5a 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -1141,6 +1141,12 @@ public: */ BINLOG_STMT_UNSAFE_NONTRANS_AFTER_TRANS, + /** + Mixing self-logging and non-self-logging engines in a statement + is unsafe. + */ + BINLOG_STMT_UNSAFE_MULTIPLE_ENGINES_AND_SELF_LOGGING_ENGINE, + /* The last element of this enumeration type. */ BINLOG_STMT_UNSAFE_COUNT };