MDEV-31033 ER_KEY_NOT_FOUND upon online COPY ALTER on a partitioned table

The row events were applied "twice": once for the ha_partition, and one
more time for the underlying storage engine.

There's no such problem in binlog/rpl, because ha_partiton::row_logging
is normally set to false.

The fix makes the events replicate only when the handler is a root handler.
We will try to *guess* this by comparing it to table->file. The same
approach is used in the MDEV-21540 fix, 231feabd. The assumption is made,
that the row methods are only called for table->file (and never for a
cloned handler), hence the assertions are added in ha_innobase and
ha_myisam to make sure that this is true at least for those engines

Also closes MDEV-31040, however the test is not included, since we have no
convenient way to construct a deterministic version.

6 files changed, 68 insertions, 5 deletions

diff --git a/mysql-test/main/alter_table_online_debug.result b/mysql-test/main/alter_table_online_debug.result
index bf4977f4f7c..b42e34d69c3 100644
--- a/mysql-test/main/alter_table_online_debug.result
+++ b/mysql-test/main/alter_table_online_debug.result
@@ -1231,6 +1231,27 @@ a
 223
 889
 drop table t1;
+#
+# MDEV-31033 ER_KEY_NOT_FOUND upon online COPY ALTER on a partitioned
+# table
+create table t (a int) partition by hash(a) partitions 2;
+insert into t values (1),(3);
+set debug_sync= 'alter_table_online_downgraded SIGNAL downgraded wait_for goon';
+alter table t force, algorithm=copy, lock=none;
+connection con1;
+set debug_sync= 'now WAIT_FOR downgraded';
+update t set a = a + 1;
+insert t values (1),(2);
+delete from t where a = 4 limit 1;
+set debug_sync= 'now SIGNAL goon';
+connection default;
+select * from t;
+a
+2
+2
+1
+drop table t;
+set debug_sync= reset;
 disconnect con1;
 #
 # End of 10.10 tests
diff --git a/mysql-test/main/alter_table_online_debug.test b/mysql-test/main/alter_table_online_debug.test
index 5b86192ca37..f8b14857a37 100644
--- a/mysql-test/main/alter_table_online_debug.test
+++ b/mysql-test/main/alter_table_online_debug.test
@@ -1418,6 +1418,29 @@ select * from t1;
 
 drop table t1;
 
+--echo #
+--echo # MDEV-31033 ER_KEY_NOT_FOUND upon online COPY ALTER on a partitioned
+--echo # table
+create table t (a int) partition by hash(a) partitions 2;
+insert into t values (1),(3);
+set debug_sync= 'alter_table_online_downgraded SIGNAL downgraded wait_for goon';
+send alter table t force, algorithm=copy, lock=none;
+
+--connection con1
+set debug_sync= 'now WAIT_FOR downgraded';
+update t set a = a + 1;
+insert t values (1),(2);
+delete from t where a = 4 limit 1;
+set debug_sync= 'now SIGNAL goon';
+
+--connection default
+--reap
+
+select * from t;
+
+drop table t;
+
+set debug_sync= reset;
 --disconnect con1
 --echo #
 --echo # End of 10.10 tests
diff --git a/sql/handler.cc b/sql/handler.cc
index 3fcbb5171d6..d8aca5bc80b 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -7280,7 +7280,7 @@ int handler::binlog_log_row(const uchar *before_record,
                                     log_func, row_logging_has_trans);
 
 #ifdef HAVE_REPLICATION
-  if (unlikely(!error && table->s->online_alter_binlog))
+  if (unlikely(!error && table->s->online_alter_binlog && is_root_handler()))
     error= binlog_log_row_online_alter(table, before_record, after_record,
                                        log_func);
 #endif // HAVE_REPLICATION
@@ -7800,7 +7800,7 @@ int handler::ha_write_row(const uchar *buf)
     DBUG_RETURN(error);
 
   /*
-    NOTE: this != table->file is true in 3 cases:
+    NOTE: is_root_handler() is false in 3 cases:
 
     1. under copy_partitions() (REORGANIZE PARTITION): that does not
        require long unique check as it does not introduce new rows or new index.
@@ -7810,7 +7810,7 @@ int handler::ha_write_row(const uchar *buf)
     3. under ha_mroonga::wrapper_write_row()
   */
 
-  if (table->s->long_unique_table && this == table->file)
+  if (table->s->long_unique_table && is_root_handler())
   {
     DBUG_ASSERT(inited == NONE || lookup_handler != this);
     if ((error= check_duplicate_long_entries(buf)))
@@ -7862,13 +7862,13 @@ int handler::ha_update_row(const uchar *old_data, const uchar *new_data)
   error= ha_check_overlaps(old_data, new_data);
 
   /*
-    NOTE: this != table->file is true under partition's ha_update_row():
+    NOTE: is_root_handler() is false under partition's ha_update_row():
     check_duplicate_long_entries_update() was already done by
     ha_partition::ha_update_row(), no need to check it again for each single
     partition. Same applies to ha_mroonga wrapper.
   */
 
-  if (!error && table->s->long_unique_table && this == table->file)
+  if (!error && table->s->long_unique_table && is_root_handler())
     error= check_duplicate_long_entries_update(new_data);
   table->status= saved_status;
 
@@ -7913,6 +7913,12 @@ int handler::ha_update_row(const uchar *old_data, const uchar *new_data)
   return error;
 }
 
+
+bool handler::is_root_handler() const
+{
+  return this == table->file;
+}
+
 /*
   Update first row. Only used by sequence tables
 */
diff --git a/sql/handler.h b/sql/handler.h
index fb0eba83c38..9b903e7ebce 100644
--- a/sql/handler.h
+++ b/sql/handler.h
@@ -3502,6 +3502,10 @@ public:
     return extra(HA_EXTRA_NO_KEYREAD);
   }
 
+protected:
+  bool is_root_handler() const;
+
+public:
   int check_collation_compatibility();
   int check_long_hash_compatibility() const;
   int ha_check_for_upgrade(HA_CHECK_OPT *check_opt);
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index f035437b1d7..94d9da3486b 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -7750,6 +7750,8 @@ ha_innobase::write_row(
 
 	DBUG_ENTER("ha_innobase::write_row");
 
+	DBUG_ASSERT(is_root_handler() || table->file->ht != innodb_hton_ptr);
+
 	trx_t*		trx = thd_to_trx(m_user_thd);
 
 	/* Validation checks before we commence write_row operation. */
@@ -8479,6 +8481,8 @@ ha_innobase::update_row(
 
 	DBUG_ENTER("ha_innobase::update_row");
 
+	DBUG_ASSERT(is_root_handler() || table->file->ht != innodb_hton_ptr);
+
 	if (is_read_only()) {
 		DBUG_RETURN(HA_ERR_TABLE_READONLY);
 	} else if (!trx_is_started(trx)) {
@@ -8653,6 +8657,8 @@ ha_innobase::delete_row(
 
 	DBUG_ENTER("ha_innobase::delete_row");
 
+	DBUG_ASSERT(is_root_handler() || table->file->ht != innodb_hton_ptr);
+
 	if (is_read_only()) {
 		DBUG_RETURN(HA_ERR_TABLE_READONLY);
 	} else if (!trx_is_started(trx)) {
diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc
index 4057c3ea859..b7fadd86a23 100644
--- a/storage/myisam/ha_myisam.cc
+++ b/storage/myisam/ha_myisam.cc
@@ -983,6 +983,7 @@ int ha_myisam::close(void)
 
 int ha_myisam::write_row(const uchar *buf)
 {
+  DBUG_ASSERT(is_root_handler() || table->file->ht != ht);
   /*
     If we have an auto_increment column and we are writing a changed row
     or a new row, then update the auto_increment value in the record.
@@ -1957,11 +1958,13 @@ bool ha_myisam::is_crashed() const
 
 int ha_myisam::update_row(const uchar *old_data, const uchar *new_data)
 {
+  DBUG_ASSERT(is_root_handler() || table->file->ht != ht);
   return mi_update(file,old_data,new_data);
 }
 
 int ha_myisam::delete_row(const uchar *buf)
 {
+  DBUG_ASSERT(is_root_handler() || table->file->ht != ht);
   return mi_delete(file,buf);
 }