MDEV-23672 Assertion `v.v_indexes.empty()' failed in dict_table_t::in…

…stant_column

dict_v_idx_t node was shared between two dict_v_col_t objects because
of wrong object copy. Replace memory plain copy with copy constructor.

Tha patch also removes n_v_indexes property and improves "page full"
judgements for trx_undo_log_v_idx().

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

@@ -413,4 +413,29 @@ DROP TABLE t;
 CREATE TABLE t (pk SERIAL, b TEXT CHARACTER SET utf8) ENGINE=InnoDB;
 ALTER TABLE t MODIFY b TEXT CHARACTER SET utf8mb4 FIRST;
 DROP TABLE t;
+#
+# MDEV-23672 Assertion `v.v_indexes.empty()' failed in dict_table_t::instant_column
+#
+create table t1 (
+col_int integer,
+col_text text not null,
+col_int_g integer generated always as (col_int) unique,
+col_text_g text generated always as (substr(col_text,1,499)) )
+engine innodb row_format = redundant;
+insert into t1 values (0, 'a', default, default);
+insert into t1 values (null, 'b', default, default);
+alter table t1 modify column col_text text null, algorithm = instant;
+insert into t1 values (1, null, default, default);
+insert into t1 values (null, null, default, default);
+update t1 set col_text= 'd';
+select * from t1;
+col_int	col_text	col_int_g	col_text_g
+0	d	0	d
+NULL	d	NULL	d
+1	d	1	d
+NULL	d	NULL	d
+check table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	check	status	OK
+drop table t1;
 SET GLOBAL innodb_purge_rseg_truncate_frequency=@save_frequency;

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

@@ -436,4 +436,23 @@ CREATE TABLE t (pk SERIAL, b TEXT CHARACTER SET utf8) ENGINE=InnoDB;
 ALTER TABLE t MODIFY b TEXT CHARACTER SET utf8mb4 FIRST;
 DROP TABLE t;
 
+--echo #
+--echo # MDEV-23672 Assertion `v.v_indexes.empty()' failed in dict_table_t::instant_column
+--echo #
+create table t1 (
+    col_int integer,
+    col_text text not null,
+    col_int_g integer generated always as (col_int) unique,
+    col_text_g text generated always as (substr(col_text,1,499)) )
+engine innodb row_format = redundant;
+insert into t1 values (0, 'a', default, default);
+insert into t1 values (null, 'b', default, default);
+alter table t1 modify column col_text text null, algorithm = instant;
+insert into t1 values (1, null, default, default);
+insert into t1 values (null, null, default, default);
+update t1 set col_text= 'd';
+select * from t1;
+check table t1;
+drop table t1;
+
 SET GLOBAL innodb_purge_rseg_truncate_frequency=@save_frequency;

storage/innobase/dict/dict0dict.cc

@@ -2146,7 +2146,6 @@ dict_index_add_col(
 	if (col->is_virtual()) {
 		dict_v_col_t*	v_col = reinterpret_cast<dict_v_col_t*>(col);
 		/* Register the index with the virtual column index list */
-		v_col->n_v_indexes++;
 		v_col->v_indexes.push_front(dict_v_idx_t(index, index->n_def));
 		col_name = dict_table_get_v_col_name_mysql(
 			table, dict_col_get_no(col));

storage/innobase/dict/dict0mem.cc

@@ -425,7 +425,6 @@ dict_mem_table_add_v_col(
 
 	/* Initialize the index list for virtual columns */
 	ut_ad(v_col->v_indexes.empty());
-	v_col->n_v_indexes = 0;
 
 	return(v_col);
 }

storage/innobase/handler/handler0alter.cc

@@ -589,8 +589,10 @@ inline bool dict_table_t::instant_column(const dict_table_t& table,
 			mem_heap_dup(heap, table.v_col_names,
 				     ulint(end - table.v_col_names)));
 		v_cols = static_cast<dict_v_col_t*>(
-			mem_heap_dup(heap, table.v_cols,
-				     table.n_v_cols * sizeof *v_cols));
+			mem_heap_alloc(heap, table.n_v_cols * sizeof(*v_cols)));
+		for (ulint i = table.n_v_cols; i--; ) {
+			new (&v_cols[i]) dict_v_col_t(table.v_cols[i]);
+		}
 	} else {
 		ut_ad(table.n_v_cols == 0);
 		v_col_names = NULL;
@@ -603,8 +605,6 @@ inline bool dict_table_t::instant_column(const dict_table_t& table,
 
 	for (unsigned i = 0; i < n_v_def; i++) {
 		dict_v_col_t& v = v_cols[i];
-		DBUG_ASSERT(v.v_indexes.empty());
-		v.n_v_indexes = 0;
 		v.base_col = static_cast<dict_col_t**>(
 			mem_heap_dup(heap, v.base_col,
 				     v.num_base * sizeof *v.base_col));
@@ -714,7 +714,6 @@ inline bool dict_table_t::instant_column(const dict_table_t& table,
 					<dict_v_col_t*>(f.col);
 				v_col->v_indexes.push_front(
 					dict_v_idx_t(index, i));
-				v_col->n_v_indexes++;
 			}
 		}
 	}
@@ -5031,7 +5030,6 @@ prepare_inplace_add_virtual(
 		ctx->add_vcol[j].v_pos = ctx->old_table->n_v_cols
 					 - ctx->num_to_drop_vcol + j;
 
-		ctx->add_vcol[j].n_v_indexes = 0;
 		/* MDEV-17468: Do this on ctx->instant_table later */
 		innodb_base_col_setup(ctx->old_table, field, &ctx->add_vcol[j]);
 		j++;

storage/innobase/include/dict0mem.h

@@ -766,9 +766,6 @@ struct dict_v_col_t{
 	/** column pos in table */
 	unsigned		v_pos:10;
 
-	/** number of indexes */
-	unsigned		n_v_indexes:12;
-
 	/** Virtual index list, and column position in the index */
 	std::forward_list<dict_v_idx_t, ut_allocator<dict_v_idx_t> >
 	v_indexes;
@@ -777,21 +774,17 @@ struct dict_v_col_t{
   @param index  index to be detached from */
   void detach(const dict_index_t &index)
   {
-    if (!n_v_indexes) return;
+    if (v_indexes.empty()) return;
     auto i= v_indexes.before_begin();
-    ut_d(unsigned n= 0);
     do {
       auto prev = i++;
       if (i == v_indexes.end())
       {
-        ut_ad(n == n_v_indexes);
         return;
       }
-      ut_ad(++n <= n_v_indexes);
       if (i->index == &index)
       {
         v_indexes.erase_after(prev);
-        n_v_indexes--;
         return;
       }
     }

storage/innobase/trx/trx0rec.cc

@@ -230,20 +230,35 @@ trx_undo_log_v_idx(
 {
 	ut_ad(pos < table->n_v_def);
 	dict_v_col_t*	vcol = dict_table_get_nth_v_col(table, pos);
-	ulint		n_idx = vcol->n_v_indexes;
 	byte*		old_ptr;
 
-	ut_ad(n_idx > 0);
+	ut_ad(!vcol->v_indexes.empty());
 
 	/* Size to reserve, max 5 bytes for each index id and position, plus
 	5 bytes for num of indexes, 2 bytes for write total length.
 	1 byte for undo log record format version marker */
-	ulint		size = n_idx * (5 + 5) + 5 + 2 + (first_v_col ? 1 : 0);
+	ulint		size = 5 + 2 + (first_v_col ? 1 : 0);
+	const ulint	avail = trx_undo_left(undo_block, ptr);
 
-	if (trx_undo_left(undo_block, ptr) < size) {
+	if (avail < size) {
 		return(NULL);
 	}
 
+	size = 0;
+	ulint n_idx = 0;
+	for (const auto& v_index : vcol->v_indexes) {
+		n_idx++;
+		/* FIXME: index->id is 64 bits! */
+		size += mach_get_compressed_size(uint32_t(v_index.index->id));
+		size += mach_get_compressed_size(v_index.nth_field);
+	}
+	size += 2 + mach_get_compressed_size(n_idx);
+
+	if (avail < size) {
+		return(NULL);
+	}
+
+
 	if (first_v_col) {
 		/* write the version marker */
 		mach_write_to_1(ptr, VIRTUAL_COL_UNDO_FORMAT_1);
@@ -259,7 +274,8 @@ trx_undo_log_v_idx(
 
 	for (const auto& v_index : vcol->v_indexes) {
 		ptr += mach_write_compressed(
-			ptr, static_cast<ulint>(v_index.index->id));
+			/* FIXME: index->id is 64 bits! */
+			ptr, uint32_t(v_index.index->id));
 
 		ptr += mach_write_compressed(ptr, v_index.nth_field);
 	}