Revert "[storage/innobase] PS-9144 : Missing rows after ALGORITHM=INPLACE ALTER under same workload as PS-9092"

This reverts commit 4614f68.

Author: inikep

mysql-test/suite/innodb/r/percona_alter_debug.result

@@ -1,30 +1,21 @@
 #
-# Test for PS-9144 : Missing rows after ALGORITHM=INPLACE ALTER under
-#                    same workload as PS-9092.
+# Test for PS-9214 : Alter table online results in "duplicate key"
+#                    error on the primary key (only index).
 #
 CREATE TABLE t1 (pk CHAR(5) PRIMARY KEY);
-INSERT INTO t1 VALUES ('aaaaa'), ('bbbbb'), ('bbbcc'), ('ccccc'), ('ddddd'), ('eeeee');
-set global innodb_purge_stop_now=ON;
-DELETE FROM t1 WHERE pk = 'bbbcc';
+INSERT INTO t1 VALUES ('aaaaa'), ('bbbbb'), ('ccccc'), ('ddddd'), ('eeeee');
 connect  con1, localhost, root,,;
 SET DEBUG='+d,ddl_buf_add_two';
 SET DEBUG_SYNC='ddl_bulk_inserter_latches_released SIGNAL latches_released WAIT_FOR go';
 # Send ALTER TABLE INPLACE which rebuilds table.
 ALTER TABLE t1 ENGINE=InnoDB, ALGORITHM=INPLACE;
 connection default;
 SET DEBUG_SYNC='now WAIT_FOR latches_released';
-SET GLOBAL innodb_purge_run_now=ON;
+INSERT INTO t1 VALUES ('ccaaa');
 SET DEBUG_SYNC='now SIGNAL go';
 connection con1;
 # Reap ALTER TABLE
-# Before the fix row 'ddddd' was missing from the table after ALTER.
-SELECT * FROM t1;
-pk
-aaaaa
-bbbbb
-ccccc
-ddddd
-eeeee
+# Before fix it failed with duplicate key error.
 SET DEBUG='-d,ddl_buf_add_two';
 disconnect con1;
 connection default;

mysql-test/suite/innodb/t/percona_alter_debug.test

@@ -1,17 +1,14 @@
 # Tests in this file use both debug-only facilities as well as debug sync point.
 --source include/have_debug.inc
 --source include/have_debug_sync.inc
+--enable_connect_log
 
 --echo #
---echo # Test for PS-9144 : Missing rows after ALGORITHM=INPLACE ALTER under
---echo #                    same workload as PS-9092.
+--echo # Test for PS-9214 : Alter table online results in "duplicate key"
+--echo #                    error on the primary key (only index).
 --echo #
---enable_connect_log
 CREATE TABLE t1 (pk CHAR(5) PRIMARY KEY);
-INSERT INTO t1 VALUES ('aaaaa'), ('bbbbb'), ('bbbcc'), ('ccccc'), ('ddddd'), ('eeeee');
-
-set global innodb_purge_stop_now=ON;
-DELETE FROM t1 WHERE pk = 'bbbcc';
+INSERT INTO t1 VALUES ('aaaaa'), ('bbbbb'), ('ccccc'), ('ddddd'), ('eeeee');
 
 --connect (con1, localhost, root,,)
 SET DEBUG='+d,ddl_buf_add_two';
@@ -21,17 +18,14 @@ SET DEBUG_SYNC='ddl_bulk_inserter_latches_released SIGNAL latches_released WAIT_
 
 --connection default
 SET DEBUG_SYNC='now WAIT_FOR latches_released';
-SET GLOBAL innodb_purge_run_now=ON;
---source include/wait_innodb_all_purged.inc
+INSERT INTO t1 VALUES ('ccaaa');
 SET DEBUG_SYNC='now SIGNAL go';
 
 --connection con1
 --echo # Reap ALTER TABLE
+--echo # Before fix it failed with duplicate key error.
 --reap
 
---echo # Before the fix row 'ddddd' was missing from the table after ALTER.
-SELECT * FROM t1;
-
 SET DEBUG='-d,ddl_buf_add_two';
 
 --disconnect con1
@@ -41,4 +35,5 @@ SET DEBUG='-d,ddl_buf_add_two';
 --echo # Cleanup.
 SET DEBUG_SYNC= 'RESET';
 DROP TABLE t1;
+
 --disable_connect_log

storage/innobase/ddl/ddl0par-scan.cc

@@ -237,7 +237,6 @@ dberr_t Parallel_cursor::scan(Builders &builders) noexcept {
             thread_ctx->get_state() != Parallel_reader::State::THREAD) {
           thread_ctx->savepoint();
           latches_released = true;
-          DEBUG_SYNC_C("ddl_bulk_inserter_latches_released");
         }
         return DB_SUCCESS;
       });

storage/innobase/row/row0pread.cc

@@ -215,27 +215,6 @@ class PCursor {
   PCursor(btr_pcur_t *pcur, mtr_t *mtr, size_t read_level)
       : m_mtr(mtr), m_pcur(pcur), m_read_level(read_level) {}
 
-  /**
-    Restore position of cursor created from Scan_ctx::Range object.
-    Adjust it if starting record of range has been purged.
-  */
-  void restore_position_for_range() noexcept {
-    /*
-      Restore position on the record or its predecessor if the record
-      was purged meanwhile. In the latter case we need to adjust position by
-      moving one step forward as predecessor belongs to another range.
-
-      We rely on return value from restore_position() to detect this scenario.
-      This can be done when saved position points to user record which is
-      always the case for saved start of Scan_ctx::Range.
-    */
-    ut_ad(m_pcur->m_rel_pos == BTR_PCUR_ON);
-
-    if (!m_pcur->restore_position(BTR_SEARCH_LEAF, m_mtr, UT_LOCATION_HERE)) {
-      page_cur_move_to_next(m_pcur->get_page_cur());
-    }
-  }
-
   /** Create a savepoint and commit the mini-transaction.*/
   void savepoint() noexcept;
 
@@ -247,6 +226,65 @@ class PCursor {
   @return DB_SUCCESS or error code. */
   [[nodiscard]] dberr_t move_to_next_block(dict_index_t *index);
 
+  /** Restore the cursor position. */
+  void restore_position() noexcept {
+    constexpr auto MODE = BTR_SEARCH_LEAF;
+    const auto relative = m_pcur->m_rel_pos;
+    auto equal = m_pcur->restore_position(MODE, m_mtr, UT_LOCATION_HERE);
+
+#ifdef UNIV_DEBUG
+    if (m_pcur->m_pos_state == BTR_PCUR_IS_POSITIONED_OPTIMISTIC) {
+      ut_ad(m_pcur->m_rel_pos == BTR_PCUR_BEFORE ||
+            m_pcur->m_rel_pos == BTR_PCUR_AFTER);
+    } else {
+      ut_ad(m_pcur->m_pos_state == BTR_PCUR_IS_POSITIONED);
+      ut_ad((m_pcur->m_rel_pos == BTR_PCUR_ON) == m_pcur->is_on_user_rec());
+    }
+#endif /* UNIV_DEBUG */
+
+    switch (relative) {
+      case BTR_PCUR_ON:
+        if (!equal) {
+          page_cur_move_to_next(m_pcur->get_page_cur());
+        }
+        break;
+
+      case BTR_PCUR_UNSET:
+      case BTR_PCUR_BEFORE_FIRST_IN_TREE:
+        ut_error;
+        break;
+
+      case BTR_PCUR_AFTER:
+      case BTR_PCUR_AFTER_LAST_IN_TREE:
+        break;
+
+      case BTR_PCUR_BEFORE:
+        /* For non-optimistic restoration:
+        The position is now set to the record before pcur->old_rec.
+
+        For optimistic restoration:
+        The position also needs to take the previous search_mode into
+        consideration. */
+        switch (m_pcur->m_pos_state) {
+          case BTR_PCUR_IS_POSITIONED_OPTIMISTIC:
+            m_pcur->m_pos_state = BTR_PCUR_IS_POSITIONED;
+            /* The cursor always moves "up" ie. in ascending order. */
+            break;
+
+          case BTR_PCUR_IS_POSITIONED:
+            if (m_pcur->is_on_user_rec()) {
+              m_pcur->move_to_next(m_mtr);
+            }
+            break;
+
+          case BTR_PCUR_NOT_POSITIONED:
+          case BTR_PCUR_WAS_POSITIONED:
+            ut_error;
+        }
+        break;
+    }
+  }
+
   /** @return the current page cursor. */
   [[nodiscard]] page_cur_t *get_page_cursor() noexcept {
     return m_pcur->get_page_cur();
@@ -310,7 +348,7 @@ void PCursor::resume() noexcept {
   /* Restore position on the record, or its predecessor if the record
   was purged meanwhile. */
 
-  m_pcur->restore_position(BTR_SEARCH_LEAF, m_mtr, UT_LOCATION_HERE);
+  restore_position();
 
   if (!m_pcur->is_after_last_on_page()) {
     /* Move to the successor of the saved record. */
@@ -588,7 +626,7 @@ dberr_t Parallel_reader::Ctx::traverse() {
   auto &from = m_range.first;
 
   PCursor pcursor(from->m_pcur, &mtr, m_scan_ctx->m_config.m_read_level);
-  pcursor.restore_position_for_range();
+  pcursor.restore_position();
 
   dberr_t err{DB_SUCCESS};