MDEV-7270: Fix replicas not honoring slave_skip_errors for 1677

mysql-test/suite/rpl/r/rpl_row_slave_skip_errors.result

@@ -0,0 +1,42 @@
+include/master-slave.inc
+[connection master]
+CREATE TABLE t (name VARCHAR(25) DEFAULT NULL);
+include/sync_slave_sql_with_master.inc
+call mtr.add_suppression("Slave SQL.*Error executing row event: .Table .test.t. doesn.t exist., Error_code: 1146");
+call mtr.add_suppression("Slave SQL.*Column 0 of table .test.t. cannot be converted from type.* Error_code: 1677");
+call mtr.add_suppression("The slave coordinator and worker threads are stopped, possibly leaving data in inconsistent state");
+call mtr.add_suppression("Got error 1 during COMMIT");
+ALTER TABLE t MODIFY name VARCHAR(255);
+connection master;
+INSERT INTO t VALUE ('Omar');
+# Sync should be successful. Slave should not stop with an error
+# ER_SLAVE_CONVERSION_FAILED. It should be up and running in spite
+# of errors as we have set slave_skip_error=1677.
+include/sync_slave_sql_with_master.inc
+include/check_slave_no_error.inc
+# Verify master has one row and slave has none, row events from master should be skipped.
+connection master;
+SELECT * FROM t;
+name
+Omar
+connection slave;
+SELECT * FROM t;
+name
+connection master;
+SELECT COUNT(*) AS master_count FROM t;
+master_count
+1
+connection slave;
+SELECT COUNT(*) AS slave_count FROM t;
+slave_count
+0
+==== Clean up ====
+include/stop_slave.inc
+RESET MASTER;
+RESET SLAVE;
+include/start_slave.inc
+connection master;
+include/sync_slave_sql_with_master.inc
+connection master;
+DROP TABLE t;
+include/rpl_end.inc

mysql-test/suite/rpl/t/rpl_row_slave_skip_errors-slave.opt

@@ -0,0 +1 @@
+--slave-skip-errors=1677

mysql-test/suite/rpl/t/rpl_row_slave_skip_errors.test

@@ -0,0 +1,67 @@
+# ==== Purpose ====
+#
+# Check that slave-skip-errors skips following errors like
+# ER_SLAVE_CONVERSION_FAILED and ER_NO_SUCH_TABLE.
+#
+# This test is adapted from the upstream mysql test for Bug#17653275, to verify the fix for Mariadb MDEV-7270.
+#
+# ==== Implementation ====
+# On slave, set slave_skip_errors=1677, so that slave skips ER_SLAVE_CONVERSION_FAILED 
+# reported during application of row based events.
+# On master, create a table t with a varchar filed of length 25. On slave
+# increase the varchar field width to 255, so that updates that are received
+# from master will fail on slave with error ER_SLAVE_CONVERSION_FAILED.
+#
+# Verify that slave doesn't break inspite of these errors.
+
+
+--source include/have_debug.inc
+--source include/have_binlog_format_row.inc
+--source include/master-slave.inc
+
+# On master create table t which contains a field named 'name' with length
+# varchar(25).
+CREATE TABLE t (name VARCHAR(25) DEFAULT NULL);
+--source include/sync_slave_sql_with_master.inc
+
+# On slave alter the name field length to varchar(255).
+call mtr.add_suppression("Slave SQL.*Error executing row event: .Table .test.t. doesn.t exist., Error_code: 1146");
+call mtr.add_suppression("Slave SQL.*Column 0 of table .test.t. cannot be converted from type.* Error_code: 1677");
+call mtr.add_suppression("The slave coordinator and worker threads are stopped, possibly leaving data in inconsistent state");
+call mtr.add_suppression("Got error 1 during COMMIT");
+ALTER TABLE t MODIFY name VARCHAR(255);
+
+connection master;
+INSERT INTO t VALUE ('Omar');
+--echo # Sync should be successful. Slave should not stop with an error
+--echo # ER_SLAVE_CONVERSION_FAILED. It should be up and running in spite
+--echo # of errors as we have set slave_skip_error=1677.
+--source include/sync_slave_sql_with_master.inc
+--source include/check_slave_no_error.inc
+
+--echo # Verify master has one row and slave has none, row events from master should be skipped.
+connection master;
+SELECT * FROM t;
+
+connection slave;
+SELECT * FROM t;
+
+connection master;
+SELECT COUNT(*) AS master_count FROM t;
+connection slave;
+SELECT COUNT(*) AS slave_count FROM t;
+
+
+--echo ==== Clean up ====
+--source include/stop_slave.inc
+RESET MASTER;
+RESET SLAVE;
+--source include/start_slave.inc
+
+connection master;
+--source include/sync_slave_sql_with_master.inc
+
+connection master;
+DROP TABLE t;
+--source include/rpl_end.inc
+

sql/log_event_old.cc

@@ -146,7 +146,9 @@ Old_rows_log_event::do_apply_event(Old_rows_log_event *ev, rpl_group_info *rgi)
         RPL_TABLE_LIST *ptr=static_cast<RPL_TABLE_LIST*>(table_list_ptr);
         DBUG_ASSERT(ptr->m_tabledef_valid);
         TABLE *conv_table;
-        if (!ptr->m_tabledef.compatible_with(thd, rgi, ptr->table, &conv_table))
+        if (ptr->m_tabledef.compatible_with(thd, rgi, ptr->table,
+                                            &conv_table) ==
+            table_def::TABLE_INCOMPATIBLE)
         {
           ev_thd->is_slave_error= 1;
           rgi->slave_close_thread_tables(ev_thd);
@@ -1445,7 +1447,9 @@ int Old_rows_log_event::do_apply_event(rpl_group_info *rgi)
         */
         RPL_TABLE_LIST *ptr=static_cast<RPL_TABLE_LIST*>(table_list_ptr);
         TABLE *conv_table;
-        if (ptr->m_tabledef.compatible_with(thd, rgi, ptr->table, &conv_table))
+        if (ptr->m_tabledef.compatible_with(thd, rgi, ptr->table,
+                                            &conv_table) ==
+            table_def::TABLE_INCOMPATIBLE)
         {
           thd->is_slave_error= 1;
           rgi->slave_close_thread_tables(thd);

sql/log_event_server.cc

@@ -296,20 +296,6 @@ inline int idempotent_error_code(int err_code)
   return (ret);
 }
 
-/**
-  Ignore error code specified on command line.
-*/
-
-inline int ignored_error_code(int err_code)
-{
-  if (use_slave_mask && bitmap_is_set(&slave_error_mask, err_code))
-  {
-    statistic_increment(slave_skipped_errors, LOCK_status);
-    return 1;
-  }
-  return err_code == ER_SLAVE_IGNORED_TABLE;
-}
-
 /*
   This function converts an engine's error to a server error.
 
@@ -5687,20 +5673,29 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi)
         RPL_TABLE_LIST *ptr= static_cast<RPL_TABLE_LIST*>(table_list_ptr);
         DBUG_ASSERT(ptr->m_tabledef_valid);
         TABLE *conv_table;
-        if (!ptr->m_tabledef.compatible_with(thd, rgi, ptr->table, &conv_table))
+        int compatible=
+            ptr->m_tabledef.compatible_with(thd, rgi, ptr->table, &conv_table);
+        if (compatible == table_def::TABLE_INCOMPATIBLE)
         {
-          DBUG_PRINT("debug", ("Table: %s.%s is not compatible with master",
-                               ptr->table->s->db.str,
-                               ptr->table->s->table_name.str));
-          /*
-            We should not honour --slave-skip-errors at this point as we are
-            having severe errors which should not be skiped.
-          */
+          DBUG_PRINT("debug",
+                     ("Table: %s.%s is not compatible with master",
+                      ptr->table->s->db.str, ptr->table->s->table_name.str));
           thd->is_slave_error= 1;
           /* remove trigger's tables */
           error= ERR_BAD_TABLE_DEF;
           goto err;
         }
+        if (compatible == table_def::TABLE_INCOMPATIBLE_IGNORED)
+        {
+          DBUG_PRINT("debug",
+                     ("Table: %s.%s is not compatible with master. Error 1677 "
+                      "is ignored, event is skipped",
+                      ptr->table->s->db.str, ptr->table->s->table_name.str));
+
+          error= 0;
+          goto err;
+        }
+
         DBUG_PRINT("debug", ("Table: %s.%s is compatible with master"
                              " - conv_table: %p",
                              ptr->table->s->db.str,
@@ -5840,7 +5835,6 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi)
       before in some other ROWS event. 
      */
     rgi->set_row_stmt_start_timestamp();
-
     THD_STAGE_INFO(thd, stage_executing);
     do
     {

sql/rpl_utility.h

@@ -58,14 +58,20 @@ class table_def
 
   ~table_def();
 
+  enum enum_compatibility_master_slave
+  {
+    TABLE_COMPATIBILE= 0,
+    TABLE_INCOMPATIBLE= 1,
+    TABLE_INCOMPATIBLE_IGNORED= 2
+  };
+
   /**
     Return the number of fields there is type data for.
 
     @return The number of fields that there is type data for.
    */
   ulong size() const { return m_size; }
 
-
   /**
     Returns internal binlog type code for one field,
     without translation to real types.
@@ -190,8 +196,9 @@ class table_def
     @retval 0  if the table definition is compatible with @c table
   */
 #ifndef MYSQL_CLIENT
-  bool compatible_with(THD *thd, rpl_group_info *rgi, TABLE *table,
-                      TABLE **conv_table_var) const;
+  enum_compatibility_master_slave
+  compatible_with(THD *thd, rpl_group_info *rgi, TABLE *table,
+                  TABLE **conv_table_var) const;
 
   /**
    Create a virtual in-memory temporary table structure.

sql/rpl_utility_server.cc

@@ -18,6 +18,7 @@
 #include <my_bit.h>
 #include "rpl_utility.h"
 #include "log_event.h"
+#include "slave.h"
 
 #if defined(MYSQL_CLIENT)
 #error MYSQL_CLIENT must not be defined here
@@ -893,7 +894,6 @@ const Type_handler *table_def::field_type_handler(uint col) const
   return Type_handler::get_handler_by_real_type(typecode);
 }
 
-
 /**
   Is the definition compatible with a table?
 
@@ -918,13 +918,15 @@ const Type_handler *table_def::field_type_handler(uint col) const
   @param tmp_table_var[out]
   Virtual temporary table for performing conversions, if necessary.
 
-  @retval true Master table is compatible with slave table.
-  @retval false Master table is not compatible with slave table.
+  @retval TABLE_COMPATIBLE Master table is compatible with slave table.
+  @retval TABLE_INCOMPATIBLE Master table is not compatible with slave table
+  and error is not ignored, abort replication.
+  @retval TABLE_INCOMPATIBLE_IGNORED Master table is not compatible with slave
+  table but error is ignored, skip event.
 */
-bool
-table_def::compatible_with(THD *thd, rpl_group_info *rgi,
-                           TABLE *table, TABLE **conv_table_var)
-  const
+table_def::enum_compatibility_master_slave
+table_def::compatible_with(THD *thd, rpl_group_info *rgi, TABLE *table,
+                           TABLE **conv_table_var) const
 {
   /*
     We only check the initial columns for the tables.
@@ -945,11 +947,12 @@ table_def::compatible_with(THD *thd, rpl_group_info *rgi,
                       field->table->s->db.str,
                       field->table->s->table_name.str,
                       field->field_name.str);
-      return false;
+      return TABLE_INCOMPATIBLE;
     }
 
     if (!h)
-      return false; // An unknown data type found in the binary log
+      return TABLE_INCOMPATIBLE; // An unknown data type found in the binary
+                                 // log
     Conv_source source(h, field_metadata(col), field->charset());
     enum_conv_type convtype= can_convert_field_to(field, source, rli,
                                                   Conv_param(m_flags));
@@ -970,7 +973,7 @@ table_def::compatible_with(THD *thd, rpl_group_info *rgi,
         */
         tmp_table= create_conversion_table(thd, rgi, table);
         if (tmp_table == NULL)
-            return false;
+          return TABLE_INCOMPATIBLE;
         /*
           Clear all fields up to, but not including, this column.
         */
@@ -983,9 +986,6 @@ table_def::compatible_with(THD *thd, rpl_group_info *rgi,
     }
     else
     {
-      DBUG_PRINT("debug", ("Checking column %d -"
-                           " field '%s' can not be converted",
-                           col, field->field_name.str));
       DBUG_ASSERT(col < size() && col < table->s->fields);
       DBUG_ASSERT(table->s->db.str && table->s->table_name.str);
       DBUG_ASSERT(table->in_use);
@@ -999,11 +999,28 @@ table_def::compatible_with(THD *thd, rpl_group_info *rgi,
       field->sql_rpl_type(&target_type);
       DBUG_ASSERT(source_type.length() > 0);
       DBUG_ASSERT(target_type.length() > 0);
+
+      if (ignored_error_code(ER_SLAVE_CONVERSION_FAILED))
+      {
+        DBUG_PRINT("debug", ("Checking column %d -"
+                             " field '%s' can not be converted. Error 1677 is "
+                             "ignored, event is skipped",
+                             col, field->field_name.str));
+        rli->report(WARNING_LEVEL, ER_SLAVE_CONVERSION_FAILED,
+                    rgi->gtid_info(), ER_THD(thd, ER_SLAVE_CONVERSION_FAILED),
+                    col, table->s->db.str, table->s->table_name.str,
+                    field->field_name.str);
+        return TABLE_INCOMPATIBLE_IGNORED;
+      }
+
+      DBUG_PRINT("debug", ("Checking column %d -"
+                           " field '%s' can not be converted",
+                           col, field->field_name.str));
       rli->report(ERROR_LEVEL, ER_SLAVE_CONVERSION_FAILED, rgi->gtid_info(),
                   ER_THD(thd, ER_SLAVE_CONVERSION_FAILED),
                   col, db_name, tbl_name,
                   source_type.c_ptr_safe(), target_type.c_ptr_safe());
-      return false;
+      return TABLE_INCOMPATIBLE;
     }
   }
 
@@ -1028,7 +1045,7 @@ table_def::compatible_with(THD *thd, rpl_group_info *rgi,
 #endif
 
   *conv_table_var= tmp_table;
-  return true;
+  return TABLE_COMPATIBILE;
 }
 
 

sql/slave.h

@@ -151,6 +151,20 @@ extern ulonglong slave_skipped_errors;
 extern const char *relay_log_index;
 extern const char *relay_log_basename;
 
+/**
+  Ignore error code specified on command line.
+*/
+
+inline int ignored_error_code(int err_code)
+{
+  if (use_slave_mask && bitmap_is_set(&slave_error_mask, err_code))
+  {
+    statistic_increment(slave_skipped_errors, LOCK_status);
+    return 1;
+  }
+  return err_code == ER_SLAVE_IGNORED_TABLE;
+}
+
 /*
   4 possible values for Master_info::slave_running and
   Relay_log_info::slave_running.