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);

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

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 ====
 

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."

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();

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
 };
 
 

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
   };