MDEV-20048 Assertion 'n < tuple->n_fields on ROLLBACK after DROP COLUMN

btr_push_update_extern_fields(): Add a parameter for the original number
of fields in the record before btr_cur_trim(). Assume that this function
will only be called for the clustered index, which is the only index
that can contain off-page columns.

trx_undo_prev_version_build(), btr_cur_pessimistic_update():
Only invoke btr_push_update_extern_fields() for the clustered index.

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

@@ -203,3 +203,35 @@ ALTER TABLE t1 MODIFY c TEXT NULL, ALGORITHM=INSTANT;
 ERROR 0A000: ALGORITHM=INSTANT is not supported for this operation. Try ALGORITHM=INPLACE
 ALTER TABLE t1 MODIFY c TEXT NULL;
 DROP TABLE t1;
+#
+# MDEV-20048 dtuple_get_nth_field(): Assertion 'n < tuple->n_fields'
+# failed on ROLLBACK after instant DROP COLUMN
+#
+CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1);
+ALTER TABLE t1 ADD b TEXT, ALGORITHM=INSTANT;
+SET @b = REPEAT('b', @@innodb_page_size / 2 + 1);
+INSERT INTO t1 VALUES(2, @b), (3, @b);
+BEGIN;
+DELETE FROM t1 WHERE a=2;
+connect  purge_control,localhost,root;
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+connection default;
+COMMIT;
+ALTER TABLE t1 DROP b, ALGORITHM=INSTANT;
+BEGIN;
+INSERT INTO t1 VALUES (2);
+connection purge_control;
+SELECT * FROM t1;
+a
+1
+2
+3
+disconnect purge_control;
+connection default;
+ROLLBACK;
+SELECT * FROM t1;
+a
+1
+3
+DROP TABLE t1;

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

@@ -216,3 +216,35 @@ ALTER TABLE t1 DROP INDEX ftidx;
 ALTER TABLE t1 MODIFY c TEXT NULL, ALGORITHM=INSTANT;
 ALTER TABLE t1 MODIFY c TEXT NULL;
 DROP TABLE t1;
+
+--echo #
+--echo # MDEV-20048 dtuple_get_nth_field(): Assertion 'n < tuple->n_fields'
+--echo # failed on ROLLBACK after instant DROP COLUMN
+--echo #
+CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1);
+ALTER TABLE t1 ADD b TEXT, ALGORITHM=INSTANT;
+SET @b = REPEAT('b', @@innodb_page_size / 2 + 1);
+INSERT INTO t1 VALUES(2, @b), (3, @b);
+BEGIN;
+DELETE FROM t1 WHERE a=2;
+
+# Stop purge so that it doesn't remove the delete-marked entry.
+connect (purge_control,localhost,root);
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+connection default;
+
+COMMIT;
+
+ALTER TABLE t1 DROP b, ALGORITHM=INSTANT;
+BEGIN;
+INSERT INTO t1 VALUES (2);
+
+connection purge_control;
+SELECT * FROM t1;
+disconnect purge_control;
+connection default;
+ROLLBACK;
+
+SELECT * FROM t1;
+DROP TABLE t1;

storage/innobase/btr/btr0cur.cc

@@ -4904,6 +4904,7 @@ btr_cur_pessimistic_update(
 	itself.  Thus the following call is safe. */
 	row_upd_index_replace_new_col_vals_index_pos(new_entry, index, update,
 						     entry_heap);
+	const ulint n = new_entry->n_fields;
 	btr_cur_trim(new_entry, index, update, thr);
 
 	/* We have to set appropriate extern storage bits in the new
@@ -4911,7 +4912,10 @@ btr_cur_pessimistic_update(
 
 	ut_ad(!page_is_comp(page) || !rec_get_node_ptr_flag(rec));
 	ut_ad(rec_offs_validate(rec, index, *offsets));
-	n_ext += btr_push_update_extern_fields(new_entry, update, entry_heap);
+	if (index->is_primary()) {
+		n_ext += btr_push_update_extern_fields(
+			new_entry, n, update, entry_heap);
+	}
 
 	if ((flags & BTR_NO_UNDO_LOG_FLAG)
 	    && rec_offs_any_extern(*offsets)) {
@@ -7246,29 +7250,33 @@ btr_cur_unmark_extern_fields(
 	}
 }
 
-/*******************************************************************//**
-Flags the data tuple fields that are marked as extern storage in the
+/** Flag the data tuple fields that are marked as extern storage in the
 update vector.  We use this function to remember which fields we must
 mark as extern storage in a record inserted for an update.
+@param[in,out]	tuple	clustered index record
+@param[in]	n	number of fields in tuple, before any btr_cur_trim()
+@param[in]	update	update vector
+@param[in,out]	heap	memory heap
 @return number of flagged external columns */
 ulint
-btr_push_update_extern_fields(
-/*==========================*/
-	dtuple_t*	tuple,	/*!< in/out: data tuple */
-	const upd_t*	update,	/*!< in: update vector */
-	mem_heap_t*	heap)	/*!< in: memory heap */
+btr_push_update_extern_fields(dtuple_t* tuple, ulint n, const upd_t* update,
+			      mem_heap_t* heap)
 {
 	ulint n_pushed = 0;
-	ulint			n;
-	const upd_field_t*	uf;
+	const upd_field_t* uf = update->fields;
+
+	ut_ad(n >= tuple->n_fields);
+	/* The clustered index record must always contain a
+	PRIMARY KEY and the system columns DB_TRX_ID,DB_ROLL_PTR. */
+	ut_ad(tuple->n_fields > DATA_ROLL_PTR);
+	compile_time_assert(DATA_ROLL_PTR == 2);
 
-	uf = update->fields;
-	n = upd_get_n_fields(update);
+	for (ulint un = upd_get_n_fields(update); un--; uf++) {
+		ut_ad(uf->field_no < n);
 
-	for (; n--; uf++) {
-		if (dfield_is_ext(&uf->new_val)) {
-			dfield_t*	field
-				= dtuple_get_nth_field(tuple, uf->field_no);
+		if (dfield_is_ext(&uf->new_val)
+		    && uf->field_no < tuple->n_fields) {
+			dfield_t* field = &tuple->fields[uf->field_no];
 
 			if (!dfield_is_ext(field)) {
 				dfield_set_ext(field);

storage/innobase/include/btr0cur.h

@@ -784,17 +784,17 @@ btr_rec_copy_externally_stored_field(
 	ulint*			len,
 	mem_heap_t*		heap);
 
-/*******************************************************************//**
-Flags the data tuple fields that are marked as extern storage in the
+/** Flag the data tuple fields that are marked as extern storage in the
 update vector.  We use this function to remember which fields we must
 mark as extern storage in a record inserted for an update.
+@param[in,out]	tuple	clustered index record
+@param[in]	n	number of fields in tuple, before any btr_cur_trim()
+@param[in]	update	update vector
+@param[in,out]	heap	memory heap
 @return number of flagged external columns */
 ulint
-btr_push_update_extern_fields(
-/*==========================*/
-	dtuple_t*	tuple,	/*!< in/out: data tuple */
-	const upd_t*	update,	/*!< in: update vector */
-	mem_heap_t*	heap)	/*!< in: memory heap */
+btr_push_update_extern_fields(dtuple_t* tuple, ulint n, const upd_t* update,
+			      mem_heap_t* heap)
 	MY_ATTRIBUTE((nonnull));
 /***********************************************************//**
 Sets a secondary index record's delete mark to the given value. This

storage/innobase/trx/trx0rec.cc

@@ -2473,7 +2473,10 @@ trx_undo_prev_version_build(
 
 		entry = row_rec_to_index_entry(
 			rec, index, offsets, &n_ext, heap);
-		n_ext += btr_push_update_extern_fields(entry, update, heap);
+		if (index->is_primary()) {
+			n_ext += btr_push_update_extern_fields(
+				entry, entry->n_fields, update, heap);
+		}
 		/* The page containing the clustered index record
 		corresponding to entry is latched in mtr.  Thus the
 		following call is safe. */