MDEV-13724 ALTER TABLE…ALGORITHM=INPLACE is modifying the source table

row_merge_read_clustered_index(): The row->fields[] could point
to a record in the clustered index page of the source table, or
to an old version of the record that was constructed in row_heap.

If the row->fields[] points to the clustered index page, then
we were modifying buffer pool data without holding appropriate
block->lock and without appropriate redo logging. The intention
was to modify a copy of the data, not the source file page,
because concurrent readers would still very much need the original
values of the DB_TRX_ID,DB_ROLL_PTR for their multi-versioning.

Either way, it is simplest to not write anything at all, and to
make row->fields[] point to the constant reset_trx_id.

3 files changed, 14 insertions, 13 deletions

diff --git a/storage/innobase/include/row0merge.h b/storage/innobase/include/row0merge.h
index 50c3361a3f9..c643a9f87bf 100644
--- a/storage/innobase/include/row0merge.h
+++ b/storage/innobase/include/row0merge.h
@@ -62,6 +62,9 @@ Created 13/06/2005 Jan Lindstrom
 // Forward declaration
 struct ib_sequence_t;
 
+/** The DB_TRX_ID,DB_ROLL_PTR values for "no history is available" */
+extern const byte reset_trx_id[DATA_TRX_ID_LEN + DATA_ROLL_PTR_LEN];
+
 /** @brief Block size for I/O operations in merge sort.
 
 The minimum is UNIV_PAGE_SIZE, or page_get_free_space_of_empty()
diff --git a/storage/innobase/row/row0log.cc b/storage/innobase/row/row0log.cc
index 42e9ac279c5..aaf880e8285 100644
--- a/storage/innobase/row/row0log.cc
+++ b/storage/innobase/row/row0log.cc
@@ -45,12 +45,6 @@ ulint onlineddl_rowlog_rows;
 ulint onlineddl_rowlog_pct_used;
 ulint onlineddl_pct_progress;
 
-/** The DB_TRX_ID,DB_ROLL_PTR values for "no history is available" */
-static const byte reset_trx_id[DATA_TRX_ID_LEN + DATA_ROLL_PTR_LEN] = {
-	0, 0, 0, 0, 0, 0,
-	0x80, 0, 0, 0, 0, 0, 0
-};
-
 /** Table row modification operations during online table rebuild.
 Delete-marked records are not copied to the rebuilt table. */
 enum row_tab_op {
diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
index 63b1e2a4105..cc4835d5587 100644
--- a/storage/innobase/row/row0merge.cc
+++ b/storage/innobase/row/row0merge.cc
@@ -62,6 +62,12 @@ float my_log2f(float n)
 # define posix_fadvise(fd, offset, len, advice) /* nothing */
 #endif /* _WIN32 */
 
+/** The DB_TRX_ID,DB_ROLL_PTR values for "no history is available" */
+const byte reset_trx_id[DATA_TRX_ID_LEN + DATA_ROLL_PTR_LEN] = {
+	0, 0, 0, 0, 0, 0,
+	0x80, 0, 0, 0, 0, 0, 0
+};
+
 /* Whether to disable file system cache */
 char	srv_disable_sort_file_cache;
 
@@ -2302,13 +2308,11 @@ end_of_index:
 			for which history is not going to be
 			available after the rebuild operation.
 			This essentially mimics row_purge_reset_trx_id(). */
-			byte* ptr = static_cast<byte*>(
-				row->fields[new_trx_id_col].data);
-
-			memset(ptr, 0, DATA_TRX_ID_LEN + DATA_ROLL_PTR_LEN);
-			ptr[DATA_TRX_ID_LEN] = 1U
-				<< (ROLL_PTR_INSERT_FLAG_POS - CHAR_BIT
-				    * (DATA_ROLL_PTR_LEN - 1));
+			row->fields[new_trx_id_col].data
+				= const_cast<byte*>(reset_trx_id);
+			row->fields[new_trx_id_col + 1].data
+				= const_cast<byte*>(reset_trx_id
+						    + DATA_TRX_ID_LEN);
 		}
 
 		if (add_autoinc != ULINT_UNDEFINED) {