MDEV-21088 Table cannot be loaded after instant ADD/DROP COLUMN

btr_cur_instant_init_low(): Accurately parse the metadata record
header for ROW_FORMAT=DYNAMIC and ROW_FORMAT=COMPACT. CHAR columns
used to be unnecessarily written as nonempty strings of bytes.

Author: dr-m

mysql-test/suite/innodb/r/instant_alter,4k.rdiff

@@ -318,8 +318,8 @@
  FROM information_schema.global_status
  WHERE variable_name = 'innodb_instant_alter_column';
  instants
--184
-+186
+-187
++189
  SET GLOBAL innodb_purge_rseg_truncate_frequency= @saved_frequency;
  #
  # MDEV-18266: Changing an index comment unnecessarily rebuilds index

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

@@ -904,6 +904,14 @@ a	b	c
 1	a	NULL
 2	bah	3
 DROP TABLE t1;
+CREATE TABLE t1(a CHAR(5) CHARACTER SET utf8 PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
+INSERT INTO t1 VALUES('barf');
+ALTER TABLE t1 ADD b INT FIRST, ALGORITHM=INSTANT;
+ALTER TABLE t1 ADD vb INT AS (b);
+SELECT * FROM t1;
+b	a	vb
+NULL	barf	NULL
+DROP TABLE t1;
 CREATE TABLE t1
 (id INT PRIMARY KEY, c2 INT UNIQUE,
 c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'),
@@ -1753,6 +1761,14 @@ a	b	c
 1	a	NULL
 2	bah	3
 DROP TABLE t1;
+CREATE TABLE t1(a CHAR(5) CHARACTER SET utf8 PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=COMPACT;
+INSERT INTO t1 VALUES('barf');
+ALTER TABLE t1 ADD b INT FIRST, ALGORITHM=INSTANT;
+ALTER TABLE t1 ADD vb INT AS (b);
+SELECT * FROM t1;
+b	a	vb
+NULL	barf	NULL
+DROP TABLE t1;
 CREATE TABLE t1
 (id INT PRIMARY KEY, c2 INT UNIQUE,
 c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'),
@@ -2602,12 +2618,20 @@ a	b	c
 1	a	NULL
 2	bah	3
 DROP TABLE t1;
+CREATE TABLE t1(a CHAR(5) CHARACTER SET utf8 PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
+INSERT INTO t1 VALUES('barf');
+ALTER TABLE t1 ADD b INT FIRST, ALGORITHM=INSTANT;
+ALTER TABLE t1 ADD vb INT AS (b);
+SELECT * FROM t1;
+b	a	vb
+NULL	barf	NULL
+DROP TABLE t1;
 disconnect analyze;
 SELECT variable_value-@old_instant instants
 FROM information_schema.global_status
 WHERE variable_name = 'innodb_instant_alter_column';
 instants
-184
+187
 SET GLOBAL innodb_purge_rseg_truncate_frequency= @saved_frequency;
 #
 # MDEV-18266: Changing an index comment unnecessarily rebuilds index

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

@@ -790,6 +790,15 @@ INSERT INTO t1 VALUES(2,'bah',3);
 SELECT * FROM t1;
 DROP TABLE t1;
 
+# MDEV-21088 Table cannot be loaded after instant ADD/DROP COLUMN
+eval CREATE TABLE t1(a CHAR(5) CHARACTER SET utf8 PRIMARY KEY) $engine;
+INSERT INTO t1 VALUES('barf');
+ALTER TABLE t1 ADD b INT FIRST, ALGORITHM=INSTANT;
+# this evicts and reloads the table definition until MDEV-17468 is fixed
+ALTER TABLE t1 ADD vb INT AS (b);
+SELECT * FROM t1;
+DROP TABLE t1;
+
 dec $format;
 let $redundant_4k= 0;
 }

storage/innobase/btr/btr0cur.cc

@@ -484,14 +484,55 @@ static dberr_t btr_cur_instant_init_low(dict_index_t* index, mtr_t* mtr)
 		/* This metadata record includes a BLOB that identifies
 		any dropped or reordered columns. */
 		ulint trx_id_offset = index->trx_id_offset;
-		if (!trx_id_offset) {
-			/* The PRIMARY KEY contains variable-length columns.
-			For the metadata record, variable-length columns are
-			always written with zero length. The DB_TRX_ID will
-			start right after any fixed-length columns. */
+		/* If !index->trx_id_offset, the PRIMARY KEY contains
+		variable-length columns. For the metadata record,
+		variable-length columns should be written with zero
+		length. However, before MDEV-21088 was fixed, for
+		variable-length encoded PRIMARY KEY column of type
+		CHAR, we wrote more than zero bytes. That is why we
+		must determine the actual length of each PRIMARY KEY
+		column.  The DB_TRX_ID will start right after any
+		PRIMARY KEY columns. */
+		ut_ad(index->n_uniq);
+
+		/* We cannot invoke rec_get_offsets() before
+		index->table->deserialise_columns(). Therefore,
+		we must duplicate some logic here. */
+		if (trx_id_offset) {
+		} else if (index->table->not_redundant()) {
+			/* PRIMARY KEY columns can never be NULL.
+			We can skip the null flag bitmap. */
+			const byte* lens = rec - (REC_N_NEW_EXTRA_BYTES + 1)
+				- index->n_core_null_bytes;
+			unsigned n_add = rec_get_n_add_field(lens);
+			ut_ad(index->n_core_fields + n_add >= index->n_fields);
+			lens -= n_add;
+
 			for (uint i = index->n_uniq; i--; ) {
-				trx_id_offset += index->fields[i].fixed_len;
+				const dict_field_t& f = index->fields[i];
+				unsigned len = f.fixed_len;
+				if (!len) {
+					len = *lens--;
+					if ((len & 0x80)
+					    && DATA_BIG_COL(f.col)) {
+						/* 1exxxxxxx xxxxxxxx */
+						len &= 0x3f;
+						len <<= 8;
+						len |= *lens--;
+					}
+				}
+				trx_id_offset += len;
 			}
+		} else if (rec_get_1byte_offs_flag(rec)) {
+			trx_id_offset = rec_1_get_field_end_info(
+				rec, index->n_uniq - 1);
+			ut_ad(!(trx_id_offset & REC_1BYTE_SQL_NULL_MASK));
+			trx_id_offset &= ~REC_1BYTE_SQL_NULL_MASK;
+		} else {
+			trx_id_offset = rec_2_get_field_end_info(
+				rec, index->n_uniq - 1);
+			ut_ad(!(trx_id_offset & REC_2BYTE_SQL_NULL_MASK));
+			trx_id_offset &= ~REC_2BYTE_SQL_NULL_MASK;
 		}
 
 		const byte* ptr = rec + trx_id_offset

storage/innobase/include/rem0rec.h

@@ -291,6 +291,24 @@ inline unsigned rec_get_n_add_field_len(ulint n_add_field)
 	return n_add_field < 0x80 ? 1 : 2;
 }
 
+/** Get the added field count in a REC_STATUS_INSTANT record.
+@param[in,out]	header	variable header of a REC_STATUS_INSTANT record
+@return	number of added fields */
+inline unsigned rec_get_n_add_field(const byte*& header)
+{
+	unsigned n_fields_add = *--header;
+	if (n_fields_add < 0x80) {
+		ut_ad(rec_get_n_add_field_len(n_fields_add) == 1);
+		return n_fields_add;
+	}
+
+	n_fields_add &= 0x7f;
+	n_fields_add |= unsigned(*--header) << 7;
+	ut_ad(n_fields_add < REC_MAX_N_FIELDS);
+	ut_ad(rec_get_n_add_field_len(n_fields_add) == 2);
+	return n_fields_add;
+}
+
 /** Set the added field count in a REC_STATUS_INSTANT record.
 @param[in,out]	header	variable header of a REC_STATUS_INSTANT record
 @param[in]	n_add	number of added fields, minus 1

storage/innobase/rem/rem0rec.cc

@@ -231,24 +231,6 @@ rec_get_n_extern_new(
 	return(n_extern);
 }
 
-/** Get the added field count in a REC_STATUS_INSTANT record.
-@param[in,out]	header	variable header of a REC_STATUS_INSTANT record
-@return	number of added fields */
-static inline unsigned rec_get_n_add_field(const byte*& header)
-{
-	unsigned n_fields_add = *--header;
-	if (n_fields_add < 0x80) {
-		ut_ad(rec_get_n_add_field_len(n_fields_add) == 1);
-		return n_fields_add;
-	}
-
-	n_fields_add &= 0x7f;
-	n_fields_add |= unsigned(*--header) << 7;
-	ut_ad(n_fields_add < REC_MAX_N_FIELDS);
-	ut_ad(rec_get_n_add_field_len(n_fields_add) == 2);
-	return n_fields_add;
-}
-
 /** Format of a leaf-page ROW_FORMAT!=REDUNDANT record */
 enum rec_leaf_format {
 	/** Temporary file record */