MDEV-25236 Online log apply fails for ROW_FORMAT=REDUNDANT tables

In other ROW_FORMAT than REDUNDANT, the InnoDB record header
size calculation depends on dict_index_t::n_core_null_bytes.

In ROW_FORMAT=REDUNDANT, the record header always is 6 bytes
plus n_fields or 2*n_fields bytes, depending on the maximum
record size. But, during online ALTER TABLE, the log records
in the temporary file always use a format similar to
ROW_FORMAT=DYNAMIC, even omitting the 5-byte fixed-length part
of the header.

While creating a temporary file record for a ROW_FORMAT=REDUNDANT
table, InnoDB must refer to dict_index_t::n_nullable.
The field dict_index_t::n_core_null_bytes is only valid for
other than ROW_FORMAT=REDUNDANT tables.

The bug does not affect MariaDB 10.3, because only
commit 7a27db778e3e5a04271568a94c75157bb6fb48f1 (MDEV-15563)
allowed an ALGORITHM=INSTANT change of a NOT NULL column to
NULL in a ROW_FORMAT=REDUNDANT table.

The fix was developed by Thirunarayanan Balathandayuthapani
and tested by Matthias Leich. The test case was simplified by me.

3 files changed, 81 insertions, 13 deletions

diff --git a/mysql-test/suite/innodb/r/instant_alter_debug.result b/mysql-test/suite/innodb/r/instant_alter_debug.result
index 9a681b77c76..359eb1fc384 100644
--- a/mysql-test/suite/innodb/r/instant_alter_debug.result
+++ b/mysql-test/suite/innodb/r/instant_alter_debug.result
@@ -443,10 +443,31 @@ SET GLOBAL innodb_limit_optimistic_insert_debug=@save_limit;
 SELECT * FROM t1;
 c2	c
 DROP TABLE t1;
+#
+# MDEV-25236 Online log apply fails for ROW_FORMAT=REDUNDANT tables
+#
+CREATE TABLE t1
+(a INT NOT NULL, b INT, c INT, d INT, e INT, f INT, g INT, h INT, i TEXT)
+ENGINE=InnoDB;
+ALTER TABLE t1 MODIFY a INT NULL;
+SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL alter WAIT_FOR go';
+ALTER TABLE t1 ADD PRIMARY KEY (a);
+connect con1,localhost,root,,;
+set DEBUG_SYNC='now WAIT_FOR alter';
+BEGIN;
+INSERT INTO t1 SET a=0, i=REPEAT('1', 10000);
+ROLLBACK;
+set DEBUG_SYNC='now SIGNAL go';
+connection default;
+disconnect con1;
+SELECT * FROM t1;
+a	b	c	d	e	f	g	h	i
+DROP TABLE t1;
+SET DEBUG_SYNC=RESET;
 # End of 10.4 tests
 SET GLOBAL innodb_purge_rseg_truncate_frequency = @save_frequency;
 SELECT variable_value-@old_instant instants
 FROM information_schema.global_status
 WHERE variable_name = 'innodb_instant_alter_column';
 instants
-32
+33
diff --git a/mysql-test/suite/innodb/t/instant_alter_debug.test b/mysql-test/suite/innodb/t/instant_alter_debug.test
index b93b9dd8f1b..10f7546cc36 100644
--- a/mysql-test/suite/innodb/t/instant_alter_debug.test
+++ b/mysql-test/suite/innodb/t/instant_alter_debug.test
@@ -512,6 +512,32 @@ SET GLOBAL innodb_limit_optimistic_insert_debug=@save_limit;
 SELECT * FROM t1;
 DROP TABLE t1;
 
+--echo #
+--echo # MDEV-25236 Online log apply fails for ROW_FORMAT=REDUNDANT tables
+--echo #
+
+CREATE TABLE t1
+(a INT NOT NULL, b INT, c INT, d INT, e INT, f INT, g INT, h INT, i TEXT)
+ENGINE=InnoDB;
+
+ALTER TABLE t1 MODIFY a INT NULL;
+
+SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL alter WAIT_FOR go';
+send ALTER TABLE t1 ADD PRIMARY KEY (a);
+connect(con1,localhost,root,,);
+set DEBUG_SYNC='now WAIT_FOR alter';
+BEGIN;
+INSERT INTO t1 SET a=0, i=REPEAT('1', 10000);
+ROLLBACK;
+set DEBUG_SYNC='now SIGNAL go';
+connection default;
+reap;
+
+disconnect con1;
+SELECT * FROM t1;
+DROP TABLE t1;
+SET DEBUG_SYNC=RESET;
+
 --echo # End of 10.4 tests
 
 SET GLOBAL innodb_purge_rseg_truncate_frequency = @save_frequency;
diff --git a/storage/innobase/rem/rem0rec.cc b/storage/innobase/rem/rem0rec.cc
index 581637be073..08682304410 100644
--- a/storage/innobase/rem/rem0rec.cc
+++ b/storage/innobase/rem/rem0rec.cc
@@ -248,6 +248,8 @@ enum rec_leaf_format {
 in ROW_FORMAT=COMPACT,DYNAMIC,COMPRESSED.
 This is a special case of rec_init_offsets() and rec_get_offsets_func().
 @tparam	mblob	whether the record includes a metadata BLOB
+@tparam	redundant_temp	whether the record belongs to a temporary file
+			of a ROW_FORMAT=REDUNDANT table
 @param[in]	rec	leaf-page record
 @param[in]	index	the index that the record belongs in
 @param[in]	n_core	number of core fields (index->n_core_fields)
@@ -255,7 +257,7 @@ This is a special case of rec_init_offsets() and rec_get_offsets_func().
 			NULL to refer to index->fields[].col->def_val
 @param[in,out]	offsets	offsets, with valid rec_offs_n_fields(offsets)
 @param[in]	format	record format */
-template<bool mblob = false>
+template<bool mblob = false, bool redundant_temp = false>
 static inline
 void
 rec_init_offsets_comp_ordinary(
@@ -286,7 +288,9 @@ rec_init_offsets_comp_ordinary(
 	const unsigned n_core_null_bytes = UNIV_UNLIKELY(index->n_core_fields
 							 != n_core)
 		? UT_BITS_IN_BYTES(unsigned(index->get_n_nullable(n_core)))
-		: index->n_core_null_bytes;
+		: (redundant_temp
+		   ? UT_BITS_IN_BYTES(index->n_nullable)
+		   : index->n_core_null_bytes);
 
 	if (mblob) {
 		ut_ad(index->is_dummy || index->table->instant);
@@ -1109,8 +1113,8 @@ rec_get_nth_field_offs_old(
 }
 
 /** Determine the size of a data tuple prefix in ROW_FORMAT=COMPACT.
-@tparam	mblob	        whether the record includes a metadata BLOB
-@tparam redundant_temp  whether to use the ROW_FORMAT=REDUNDANT format
+@tparam	mblob		whether the record includes a metadata BLOB
+@tparam redundant_temp	whether to use the ROW_FORMAT=REDUNDANT format
 @param[in]	index		record descriptor; dict_table_is_comp()
 				is assumed to hold, even if it doesn't
 @param[in]	dfield		array of data fields
@@ -1157,7 +1161,9 @@ rec_get_converted_size_comp_prefix_low(
 						  - n_core_fields);
 	} else {
 		ut_ad(n_fields <= n_core_fields);
-		extra_size += index->n_core_null_bytes;
+		extra_size += redundant_temp
+			? UT_BITS_IN_BYTES(index->n_nullable)
+			: index->n_core_null_bytes;
 	}
 
 	ulint data_size = 0;
@@ -1837,10 +1843,19 @@ rec_init_offsets_temp(
 	if it was emptied during an ALTER TABLE operation. */
 	ut_ad(index->n_core_fields == n_core || !index->is_instant());
 	ut_ad(index->n_core_fields >= n_core);
-	rec_init_offsets_comp_ordinary(rec, index, offsets, n_core, def_val,
-				       status == REC_STATUS_INSTANT
-				       ? REC_LEAF_TEMP_INSTANT
-				       : REC_LEAF_TEMP);
+	if (index->table->not_redundant()) {
+		rec_init_offsets_comp_ordinary(
+			rec, index, offsets, n_core, def_val,
+			status == REC_STATUS_INSTANT
+			? REC_LEAF_TEMP_INSTANT
+			: REC_LEAF_TEMP);
+	} else {
+		rec_init_offsets_comp_ordinary<false, true>(
+			rec, index, offsets, n_core, def_val,
+			status == REC_STATUS_INSTANT
+			? REC_LEAF_TEMP_INSTANT
+			: REC_LEAF_TEMP);
+	}
 }
 
 /** Determine the offset to each field in temporary file.
@@ -1855,9 +1870,15 @@ rec_init_offsets_temp(
 	rec_offs*		offsets)
 {
 	ut_ad(!index->is_instant());
-	rec_init_offsets_comp_ordinary(rec, index, offsets,
-				       index->n_core_fields, NULL,
-				       REC_LEAF_TEMP);
+	if (index->table->not_redundant()) {
+		rec_init_offsets_comp_ordinary(
+			rec, index, offsets,
+			index->n_core_fields, NULL, REC_LEAF_TEMP);
+	} else {
+		rec_init_offsets_comp_ordinary<false, true>(
+			rec, index, offsets,
+			index->n_core_fields, NULL, REC_LEAF_TEMP);
+	}
 }
 
 /** Convert a data tuple prefix to the temporary file format.