MDEV-18033/MDEV-18034: Assertion failed on NOT NULL removal

innobase_instant_try(): Only try to update the hidden metadata
record if the number of columns is changing (increasing) or
a metadata BLOB is being added due to permuting or dropping columns
for the first time.

dict_table_t::instant_column(), ha_innobase_inplace_ctx::instant_column():
Return whether the metadata record needs to be updated.

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

@@ -624,6 +624,11 @@ CREATE TABLE t1 (a INT, b INT, c INT NOT NULL, d INT,
 e INT, f INT, g INT, h INT, j INT) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
 ALTER TABLE t1 MODIFY COLUMN c INT, MODIFY COLUMN a INT AFTER b;
 DROP TABLE t1;
+CREATE TABLE t1 (a INT NOT NULL) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
+INSERT INTO t1 VALUES (1);
+ALTER TABLE t1 ADD COLUMN b INT;
+ALTER TABLE t1 MODIFY COLUMN a INT NULL;
+DROP TABLE t1;
 CREATE TABLE t1
 (id INT PRIMARY KEY, c2 INT UNIQUE,
 c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'),
@@ -1194,6 +1199,11 @@ CREATE TABLE t1 (a INT, b INT, c INT NOT NULL, d INT,
 e INT, f INT, g INT, h INT, j INT) ENGINE=InnoDB ROW_FORMAT=COMPACT;
 ALTER TABLE t1 MODIFY COLUMN c INT, MODIFY COLUMN a INT AFTER b;
 DROP TABLE t1;
+CREATE TABLE t1 (a INT NOT NULL) ENGINE=InnoDB ROW_FORMAT=COMPACT;
+INSERT INTO t1 VALUES (1);
+ALTER TABLE t1 ADD COLUMN b INT;
+ALTER TABLE t1 MODIFY COLUMN a INT NULL;
+DROP TABLE t1;
 CREATE TABLE t1
 (id INT PRIMARY KEY, c2 INT UNIQUE,
 c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'),
@@ -1764,10 +1774,15 @@ CREATE TABLE t1 (a INT, b INT, c INT NOT NULL, d INT,
 e INT, f INT, g INT, h INT, j INT) ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
 ALTER TABLE t1 MODIFY COLUMN c INT, MODIFY COLUMN a INT AFTER b;
 DROP TABLE t1;
+CREATE TABLE t1 (a INT NOT NULL) ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
+INSERT INTO t1 VALUES (1);
+ALTER TABLE t1 ADD COLUMN b INT;
+ALTER TABLE t1 MODIFY COLUMN a INT 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
-121
+125
 SET GLOBAL innodb_purge_rseg_truncate_frequency= @saved_frequency;

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

@@ -503,6 +503,13 @@ e INT, f INT, g INT, h INT, j INT) $engine;
 ALTER TABLE t1 MODIFY COLUMN c INT, MODIFY COLUMN a INT AFTER b;
 DROP TABLE t1;
 
+# MDEV-18033/MDEV-18034 Failing assertion on ALTER
+eval CREATE TABLE t1 (a INT NOT NULL) $engine;
+INSERT INTO t1 VALUES (1);
+ALTER TABLE t1 ADD COLUMN b INT;
+ALTER TABLE t1 MODIFY COLUMN a INT NULL;
+DROP TABLE t1;
+
 dec $format;
 }
 disconnect analyze;

storage/innobase/handler/handler0alter.cc

@@ -497,8 +497,9 @@ inline void dict_index_t::instant_add_field(const dict_index_t& instant)
 
 /** Adjust table metadata for instant ADD/DROP/reorder COLUMN.
 @param[in]	table	altered table (with dropped columns)
-@param[in]	col_map	mapping from cols[] and v_cols[] to table */
-inline void dict_table_t::instant_column(const dict_table_t& table,
+@param[in]	col_map	mapping from cols[] and v_cols[] to table
+@return		whether the metadata record must be updated */
+inline bool dict_table_t::instant_column(const dict_table_t& table,
 					 const ulint* col_map)
 {
 	DBUG_ASSERT(!table.cached);
@@ -608,10 +609,16 @@ inline void dict_table_t::instant_column(const dict_table_t& table,
 	}
 
 	dict_index_t* index = dict_table_get_first_index(this);
-
-	index->instant_add_field(*dict_table_get_first_index(&table));
+	bool metadata_changed;
+	{
+		const dict_index_t& i = *dict_table_get_first_index(&table);
+		metadata_changed = i.n_fields > index->n_fields;
+		ut_ad(i.n_fields >= index->n_fields);
+		index->instant_add_field(i);
+	}
 
 	if (instant || table.instant) {
+		const auto old_instant = instant;
 		/* FIXME: add instant->heap, and transfer ownership here */
 		if (!instant) {
 			instant = new (mem_heap_zalloc(heap, sizeof *instant))
@@ -631,6 +638,15 @@ inline void dict_table_t::instant_column(const dict_table_t& table,
 		}
 
 		init_instant<true>(table);
+
+		if (!metadata_changed) {
+			metadata_changed = !old_instant
+				|| memcmp(old_instant->field_map,
+					  instant->field_map,
+					  (index->n_fields
+					   - index->first_user_field())
+					  * sizeof *old_instant->field_map);
+		}
 	}
 
 	while ((index = dict_table_get_next_index(index)) != NULL) {
@@ -678,6 +694,7 @@ inline void dict_table_t::instant_column(const dict_table_t& table,
 
 	n_cols = table.n_cols;
 	n_v_cols = table.n_v_cols;
+	return metadata_changed;
 }
 
 /** Find the old column number for the given new column position.
@@ -1024,13 +1041,14 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx
 					       first_alter_pos);
 	}
 
-	/** Adjust table metadata for instant ADD/DROP/reorder COLUMN. */
-	void instant_column()
+	/** Adjust table metadata for instant ADD/DROP/reorder COLUMN.
+	@return whether the metadata record must be updated */
+	bool instant_column()
 	{
 		DBUG_ASSERT(is_instant());
 		DBUG_ASSERT(old_n_fields
 			    == old_table->indexes.start->n_fields);
-		old_table->instant_column(*instant_table, col_map);
+		return old_table->instant_column(*instant_table, col_map);
 	}
 
 	/** Revert prepare_instant() if the transaction is rolled back. */
@@ -5413,7 +5431,7 @@ static bool innobase_instant_try(
 	const dict_col_t* old_cols = user_table->cols;
 	DBUG_ASSERT(user_table->n_cols == ctx->old_n_cols);
 
-	ctx->instant_column();
+	const bool metadata_changed = ctx->instant_column();
 
 	DBUG_ASSERT(index->n_fields >= n_old_fields);
 	/* Release the page latch. Between this and the next
@@ -5598,6 +5616,10 @@ static bool innobase_instant_try(
 			goto empty_table;
 		}
 
+		if (!metadata_changed) {
+			goto func_exit;
+		}
+
 		/* Ensure that the root page is in the correct format. */
 		buf_block_t* root = btr_root_block_get(index, RW_X_LATCH,
 						       &mtr);

storage/innobase/include/dict0mem.h

@@ -1710,8 +1710,9 @@ struct dict_table_t {
 
 	/** Adjust table metadata for instant ADD/DROP/reorder COLUMN.
 	@param[in]	table	table on which prepare_instant() was invoked
-	@param[in]	col_map	mapping from cols[] and v_cols[] to table */
-	inline void instant_column(const dict_table_t& table,
+	@param[in]	col_map	mapping from cols[] and v_cols[] to table
+	@return		whether the metadata record must be updated */
+	inline bool instant_column(const dict_table_t& table,
 				   const ulint* col_map);
 
 	/** Roll back instant_column().