MDEV-33087 ALTER TABLE...ALGORITHM=COPY should build indexes more eff…

…iciently

- During copy algorithm, InnoDB should use bulk insert operation
for row by row insert operation. By doing this, copy algorithm
can effectively build indexes. This optimization is disabled
for temporary table, versioning table and table which has
foreign key relation.

Introduced the variable innodb_alter_copy_bulk to allow
the bulk insert operation for copy alter operation
inside InnoDB. This is enabled by default

ha_innobase::extra(): HA_EXTRA_END_ALTER_COPY mode tries to apply
the buffered bulk insert operation, updates the non-persistent
table stats.

row_merge_bulk_t::write_to_index(): Update stat_n_rows after
applying the bulk insert operation

row_ins_clust_index_entry_low(): In case of copy algorithm,
switch to bulk insert operation.

copy_data_error_ignore(): Handles the error while copying
the data from source to target file.

mysql-test/main/rowid_filter_innodb.result

@@ -1986,7 +1986,7 @@ ANALYZE
  "r_table_time_ms": "REPLACED",
  "r_other_time_ms": "REPLACED",
  "r_engine_stats": {
- "pages_accessed": 3
+ "pages_accessed": 2
  },
  "filtered": "REPLACED",
  "r_filtered": 66.66666667,
@@ -2140,7 +2140,7 @@ ANALYZE
  "r_table_time_ms": "REPLACED",
  "r_other_time_ms": "REPLACED",
  "r_engine_stats": {
- "pages_accessed": 3
+ "pages_accessed": 2
  },
  "filtered": "REPLACED",
  "r_filtered": 66.66666667,

mysql-test/suite/innodb/r/alter_copy_bulk,OFF.rdiff

@@ -0,0 +1,11 @@
+--- bulk_copy_alter.result
++++ bulk_copy_alter,non_bulk_alter_copy.result
+@@ -5,7 +5,7 @@
+ INSERT INTO t1 SELECT repeat('b', 200), seq FROM seq_3_to_65536;
+ ALTER TABLE t1 ADD INDEX(f2);
+ ALTER TABLE t1 ADD PRIMARY KEY(f1(2));
+-ERROR 23000: Duplicate entry 'bb' for key 'PRIMARY'
++ERROR 23000: Duplicate entry 'aa' for key 'PRIMARY'
+ INSERT INTO t1 VALUES(repeat('a', 200), 1);
+ ALTER TABLE t1 ADD UNIQUE KEY(f2);
+ ERROR 23000: Duplicate entry '1' for key 'f2_2'

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

@@ -0,0 +1,24 @@
+SET @@alter_algorithm=COPY;
+CREATE TABLE t1(f1 CHAR(200), f2 INT NOT NULL)engine=InnoDB;
+INSERT INTO t1 SELECT repeat('a', 200), seq FROM seq_1_to_2;
+ALTER TABLE t1 FORCE;
+INSERT INTO t1 SELECT repeat('b', 200), seq FROM seq_3_to_65536;
+ALTER TABLE t1 ADD INDEX(f2);
+ALTER TABLE t1 ADD PRIMARY KEY(f1(2));
+ERROR 23000: Duplicate entry 'bb' for key 'PRIMARY'
+INSERT INTO t1 VALUES(repeat('a', 200), 1);
+ALTER TABLE t1 ADD UNIQUE KEY(f2);
+ERROR 23000: Duplicate entry '1' for key 'f2_2'
+ALTER IGNORE TABLE t1 MODIFY f1 CHAR(200) NOT NULL;
+CREATE TABLE t2(f1 INT NOT NULL,
+FOREIGN KEY(f1) REFERENCES t1(f2))ENGINE=InnoDB;
+INSERT INTO t2 VALUES(1);
+ALTER TABLE t2 FORCE;
+DROP TABLE t2, t1;
+CREATE TABLE t1 (f1 INT, f2 INT) ENGINE=InnoDB PARTITION BY HASH(f1) PARTITIONS 2;
+INSERT INTO t1 VALUES(1, 1);
+INSERT INTO t1 SELECT seq, seq * 2 FROM seq_1_to_2;
+ALTER TABLE t1 FORCE;
+INSERT INTO t1 SELECT seq, seq * 2 FROM seq_3_to_65536;
+ALTER TABLE t1 ADD INDEX(f2);
+DROP TABLE t1;

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

@@ -1119,4 +1119,26 @@ test.collections check status OK
 disconnect con1;
 DROP TABLE binaries, collections;
 # End of 10.6 tests
+CREATE TABLE t1
+(
+f1 VARCHAR(32)BINARY NOT NULL,
+f2 VARCHAR(32)BINARY NOT NULL,
+PRIMARY KEY (f1),
+INDEX(f2)
+) ENGINE=InnoDB;
+INSERT INTO t1 VALUES('MySQL', 'InnoDB'), ('MariaDB', 'NDB');
+CREATE TABLE t2
+(
+f1 VARCHAR(32)BINARY NOT NULL,
+f2 VARCHAR(255)BINARY NOT NULL,
+f3 int, PRIMARY KEY (f1), INDEX(f1), INDEX(f2)
+) ENGINE=InnoDB;
+INSERT INTO t2 VALUES('MySQL', 'MySQL', 1),
+('MariaDB', 'MariaDB', 1);
+ALTER TABLE t1 ADD FOREIGN KEY (f1) REFERENCES t2 (f2);
+ALTER TABLE t2 ADD FOREIGN KEY (f2) REFERENCES t2 (f2),
+ADD UNIQUE INDEX(f3);
+ERROR HY000: Cannot delete rows from table which is parent in a foreign key constraint 't1_ibfk_1' of table 't1'
+drop table t1, t2;
 SET GLOBAL innodb_stats_persistent = @save_stats_persistent;
+# End of 10.11 tests

mysql-test/suite/innodb/t/alter_copy_bulk.combinations

@@ -0,0 +1,5 @@
+[ON]
+--innodb_alter_copy_bulk=ON
+
+[OFF]
+--innodb_alter_copy_bulk=OFF

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

@@ -0,0 +1,44 @@
+--source include/have_innodb.inc
+--source include/have_partition.inc
+--source include/have_sequence.inc
+SET @@alter_algorithm=COPY;
+
+CREATE TABLE t1(f1 CHAR(200), f2 INT NOT NULL)engine=InnoDB;
+INSERT INTO t1 SELECT repeat('a', 200), seq FROM seq_1_to_2;
+# Buffer fits in the memory
+ALTER TABLE t1 FORCE;
+
+# Insert more entries
+INSERT INTO t1 SELECT repeat('b', 200), seq FROM seq_3_to_65536;
+# Alter should use temporary file for sorting
+ALTER TABLE t1 ADD INDEX(f2);
+
+# Error while buffering the insert operation
+--error ER_DUP_ENTRY
+ALTER TABLE t1 ADD PRIMARY KEY(f1(2));
+
+INSERT INTO t1 VALUES(repeat('a', 200), 1);
+# Error while applying the bulk insert operation
+--error ER_DUP_ENTRY
+ALTER TABLE t1 ADD UNIQUE KEY(f2);
+
+# Ignore shouldn't go through bulk operation
+ALTER IGNORE TABLE t1 MODIFY f1 CHAR(200) NOT NULL;
+
+CREATE TABLE t2(f1 INT NOT NULL,
+ FOREIGN KEY(f1) REFERENCES t1(f2))ENGINE=InnoDB;
+INSERT INTO t2 VALUES(1);
+# Bulk operation shouldn't happen because of foreign key constraints
+ALTER TABLE t2 FORCE;
+DROP TABLE t2, t1;
+
+CREATE TABLE t1 (f1 INT, f2 INT) ENGINE=InnoDB PARTITION BY HASH(f1) PARTITIONS 2;
+INSERT INTO t1 VALUES(1, 1);
+INSERT INTO t1 SELECT seq, seq * 2 FROM seq_1_to_2;
+# Buffer fits in the memory
+ALTER TABLE t1 FORCE;
+# Insert more entries
+INSERT INTO t1 SELECT seq, seq * 2 FROM seq_3_to_65536;
+# Alter should use temporary file for sorting
+ALTER TABLE t1 ADD INDEX(f2);
+DROP TABLE t1;

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

@@ -1186,6 +1186,30 @@ DROP TABLE binaries, collections;
 
 --echo # End of 10.6 tests
 
+CREATE TABLE t1
+(
+f1 VARCHAR(32)BINARY NOT NULL,
+f2 VARCHAR(32)BINARY NOT NULL,
+PRIMARY KEY (f1),
+INDEX(f2)
+) ENGINE=InnoDB;
+INSERT INTO t1 VALUES('MySQL', 'InnoDB'), ('MariaDB', 'NDB');
+
+CREATE TABLE t2
+(
+f1 VARCHAR(32)BINARY NOT NULL,
+f2 VARCHAR(255)BINARY NOT NULL,
+f3 int, PRIMARY KEY (f1), INDEX(f1), INDEX(f2)
+) ENGINE=InnoDB;
+INSERT INTO t2 VALUES('MySQL', 'MySQL', 1),
+ ('MariaDB', 'MariaDB', 1);
+ALTER TABLE t1 ADD FOREIGN KEY (f1) REFERENCES t2 (f2);
+# MDEV-33927 TODO: change the warning message
+--error ER_FK_CANNOT_DELETE_PARENT
+ALTER TABLE t2 ADD FOREIGN KEY (f2) REFERENCES t2 (f2),
+ADD UNIQUE INDEX(f3);
+drop table t1, t2;
 SET GLOBAL innodb_stats_persistent = @save_stats_persistent;
 
+--echo # End of 10.11 tests
 --source include/wait_until_count_sessions.inc

mysql-test/suite/sys_vars/r/sysvars_innodb.result

@@ -55,6 +55,18 @@ NUMERIC_BLOCK_SIZE 0
 ENUM_VALUE_LIST NULL
 READ_ONLY YES
 COMMAND_LINE_ARGUMENT OPTIONAL
+VARIABLE_NAME INNODB_ALTER_COPY_BULK
+SESSION_VALUE NULL
+DEFAULT_VALUE ON
+VARIABLE_SCOPE GLOBAL
+VARIABLE_TYPE BOOLEAN
+VARIABLE_COMMENT Allow bulk insert operation for copy alter operation
+NUMERIC_MIN_VALUE NULL
+NUMERIC_MAX_VALUE NULL
+NUMERIC_BLOCK_SIZE NULL
+ENUM_VALUE_LIST OFF,ON
+READ_ONLY NO
+COMMAND_LINE_ARGUMENT NONE
 VARIABLE_NAME INNODB_AUTOEXTEND_INCREMENT
 SESSION_VALUE NULL
 DEFAULT_VALUE 64

sql/sql_insert.cc

@@ -4335,7 +4335,16 @@ bool select_insert::prepare_eof()
  if (info.ignore || info.handle_duplicates != DUP_ERROR)
  if (table->file->ha_table_flags() & HA_DUPLICATE_POS)
  table->file->ha_rnd_end();
- table->file->extra(HA_EXTRA_END_ALTER_COPY);
+ if (error <= 0)
+ {
+ error= table->file->extra(HA_EXTRA_END_ALTER_COPY);
+ if (error == HA_ERR_FOUND_DUPP_KEY)
+ {
+ uint key_nr= table->file->get_dup_key(error);
+ if ((int)key_nr >= 0 && key_nr < table->s->keys)
+ print_keydup_error(table, &table->key_info[key_nr], MYF(0));
+ }
+ }
  table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
  table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE);
 

sql/sql_table.cc

@@ -11766,6 +11766,60 @@ bool mysql_trans_commit_alter_copy_data(THD *thd)
  DBUG_RETURN(error);
 }
 
+/** Handle the error when copying data from source to target table.
+@param error error code
+@param ignore alter ignore statement
+@param to target table handler
+@param thd Mysql Thread
+@param alter_ctx Runtime context for alter statement
+@retval false in case of error
+@retval true in case of skipping the row and continue alter operation */
+static bool
+copy_data_error_ignore(int &error, bool ignore, TABLE *to,
+ THD *thd, Alter_table_ctx *alter_ctx)
+{
+ if (to->file->is_fatal_error(error, HA_CHECK_DUP))
+ {
+ /* Not a duplicate key error. */
+ to->file->print_error(error, MYF(0));
+ error = 1;
+func_exit:
+ return false;
+ }
+ /* Duplicate key error. */
+ if (unlikely(alter_ctx->fk_error_if_delete_row))
+ {
+ /* We are trying to omit a row from the table which serves
+ as parent in a foreign key. This might have broken
+ referential integrity so emit an error. Note that we
+ can't ignore this error even if we are
+ executing ALTER IGNORE TABLE. IGNORE allows to skip rows, but
+ doesn't allow to break unique or foreign key constraints, */
+ my_error(ER_FK_CANNOT_DELETE_PARENT, MYF(0),
+ alter_ctx->fk_error_id,
+ alter_ctx->fk_error_table);
+ goto func_exit;
+ }
+ if (ignore)
+ return true;
+ /* Ordinary ALTER TABLE. Report duplicate key error. */
+ uint key_nr= to->file->get_dup_key(error);
+ if (key_nr <= MAX_KEY)
+ {
+ const char *err_msg= ER_THD(thd, ER_DUP_ENTRY_WITH_KEY_NAME);
+ if (key_nr == 0 && to->s->keys > 0 &&
+ (to->key_info[0].key_part[0].field->flags &
+ AUTO_INCREMENT_FLAG))
+ err_msg= ER_THD(thd, ER_DUP_ENTRY_AUTOINCREMENT_CASE);
+ print_keydup_error(to,
+ key_nr >= to->s->keys ? NULL :
+ &to->key_info[key_nr],
+ err_msg, MYF(0));
+ }
+ else
+ to->file->print_error(error, MYF(0));
+ goto func_exit;
+}
 
 static int
 copy_data_between_tables(THD *thd, TABLE *from, TABLE *to, bool ignore,
@@ -12027,58 +12081,11 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to, bool ignore,
  to->auto_increment_field_not_null= FALSE;
  if (unlikely(error))
  {
- if (to->file->is_fatal_error(error, HA_CHECK_DUP))
- {
- /* Not a duplicate key error. */
- to->file->print_error(error, MYF(0));
- error= 1;
- break;
- }
- else
- {
- /* Duplicate key error. */
- if (unlikely(alter_ctx->fk_error_if_delete_row))
- {
- /*
- We are trying to omit a row from the table which serves as parent
- in a foreign key. This might have broken referential integrity so
- emit an error. Note that we can't ignore this error even if we are
- executing ALTER IGNORE TABLE. IGNORE allows to skip rows, but
- doesn't allow to break unique or foreign key constraints,
- */
- my_error(ER_FK_CANNOT_DELETE_PARENT, MYF(0),
- alter_ctx->fk_error_id,
- alter_ctx->fk_error_table);
- break;
- }
-
- if (ignore)
- {
- /* This ALTER IGNORE TABLE. Simply skip row and continue. */
- to->file->restore_auto_increment(prev_insert_id);
- delete_count++;
- }
- else
- {
- /* Ordinary ALTER TABLE. Report duplicate key error. */
- uint key_nr= to->file->get_dup_key(error);
- if ((int) key_nr >= 0)
- {
- const char *err_msg= ER_THD(thd, ER_DUP_ENTRY_WITH_KEY_NAME);
- if (key_nr == 0 && to->s->keys > 0 &&
- (to->key_info[0].key_part[0].field->flags &
- AUTO_INCREMENT_FLAG))
- err_msg= ER_THD(thd, ER_DUP_ENTRY_AUTOINCREMENT_CASE);
- print_keydup_error(to,
- key_nr >= to->s->keys ? NULL :
- &to->key_info[key_nr],
- err_msg, MYF(0));
- }
- else
- to->file->print_error(error, MYF(0));
- break;
- }
- }
+ if (!copy_data_error_ignore(error, ignore, to, thd, alter_ctx))
+ break;
+ DBUG_ASSERT(ignore);
+ to->file->restore_auto_increment(prev_insert_id);
+ delete_count++;
  }
  else
  {
@@ -12105,9 +12112,15 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to, bool ignore,
  error= 1;
  }
  bulk_insert_started= 0;
- if (!ignore)
- to->file->extra(HA_EXTRA_END_ALTER_COPY);
-
+ if (!ignore && error <= 0)
+ {
+ int alt_error= to->file->extra(HA_EXTRA_END_ALTER_COPY);
+ if (alt_error > 0)
+ {
+ error= alt_error;
+ copy_data_error_ignore(error, false, to, thd, alter_ctx);
+ }
+ }
  cleanup_done= 1;
  to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
 

storage/innobase/handler/ha_innodb.cc

@@ -15754,6 +15754,26 @@ ha_innobase::extra(
  break;
  case HA_EXTRA_END_ALTER_COPY:
  trx = check_trx_exists(ha_thd());
+ if (m_prebuilt->table->skip_alter_undo) {
+ if (dberr_t err= trx->bulk_insert_apply()) {
+ m_prebuilt->table->skip_alter_undo = 0;
+ return convert_error_code_to_mysql(
+ err,
+ m_prebuilt->table->flags,
+ trx->mysql_thd);
+ }
+
+ trx->end_bulk_insert(*m_prebuilt->table);
+ trx->bulk_insert = false;
+ /* During copy alter operation, InnoDB
+ updates the stats only for non-persistent
+ tables. */
+ if (!dict_stats_is_persistent_enabled(
+ m_prebuilt->table)) {
+ dict_stats_update_if_needed(
+ m_prebuilt->table, *trx);
+ }
+ }
  m_prebuilt->table->skip_alter_undo = 0;
  if (!m_prebuilt->table->is_temporary()
  && !high_level_read_only) {
@@ -19672,6 +19692,10 @@ static MYSQL_SYSVAR_BOOL(force_primary_key,
  "Do not allow creating a table without primary key (off by default)",
  NULL, NULL, FALSE);
 
+static MYSQL_SYSVAR_BOOL(alter_copy_bulk, innodb_alter_copy_bulk,
+ PLUGIN_VAR_NOCMDARG,
+ "Allow bulk insert operation for copy alter operation", NULL, NULL, TRUE);
+
 const char *page_compression_algorithms[]= { "none", "zlib", "lz4", "lzo", "lzma", "bzip2", "snappy", 0 };
 static TYPELIB page_compression_algorithms_typelib=
 {
@@ -19906,6 +19930,7 @@ static struct st_mysql_sys_var* innobase_system_variables[]= {
  MYSQL_SYSVAR(saved_page_number_debug),
 #endif /* UNIV_DEBUG */
  MYSQL_SYSVAR(force_primary_key),
+ MYSQL_SYSVAR(alter_copy_bulk),
  MYSQL_SYSVAR(fatal_semaphore_wait_threshold),
  /* Table page compression feature */
  MYSQL_SYSVAR(compression_default),