Do not log ALTER table to ddl log for REPAIR

REPAIR of InnoDB tables was logging ALTER TABLE and REPAIR to ddl log. ALTER TABLE contained the new tableid and REPAIR, wrongly, contained the old rowid. Now only REPAIR is logged ddl.log changes: REPAIR TABLE and OPTIMIZE TABLE that are done through ALTER TABLE will now contain the old and new table id. If not done through ALTER TABLE, only the current rowid will be shown (as before).

Author: montywi

mysql-test/main/backup_log.result

@@ -171,9 +171,8 @@ ALTER,InnoDB,0,test,t1_innodb,id: 3,InnoDB,0,test,t1_innodb,id: 4
 RENAME,InnoDB,0,test,t1_innodb,id: 4,InnoDB,0,test,t2_innodb,id: 4
 RENAME,InnoDB,0,test,t2_innodb,id: 4,InnoDB,0,test,t1_innodb,id: 4
 TRUNCATE,InnoDB,0,test,t1_innodb,id: 4,,0,,,
-ALTER,InnoDB,0,test,t1_innodb,id: 4,InnoDB,0,test,t1_innodb,id: 5
-repair,InnoDB,0,test,t1_innodb,id: 4,,0,,,
-ALTER,InnoDB,0,test,t1_innodb,id: 5,InnoDB,0,test,t1_innodb,id: 6
+repair,InnoDB,0,test,t1_innodb,id: 4,,0,test,t1_innodb,id: 5
+optimize,InnoDB,0,test,t1_innodb,id: 5,,0,test,t1_innodb,id: 6
 DROP,InnoDB,0,test,t1_innodb,id: 6,,0,,,
 CREATE,MyISAM,0,test,t20,id: 7,,0,,,
 DROP,MyISAM,0,test,t20,id: 7,,0,,,

mysql-test/suite/parts/r/backup_log.result

@@ -198,7 +198,7 @@ RENAME,InnoDB,1,test,t1_innodb,id: 4,InnoDB,1,test,t2_innodb,id: 4
 RENAME,InnoDB,1,test,t2_innodb,id: 4,InnoDB,1,test,t1_innodb,id: 4
 TRUNCATE,InnoDB,1,test,t1_innodb,id: 4,,0,,,
 repair,InnoDB,1,test,t1_innodb,id: 4,,0,,,
-ALTER,InnoDB,1,test,t1_innodb,id: 4,InnoDB,1,test,t1_innodb,id: 5
+optimize,InnoDB,1,test,t1_innodb,id: 4,,0,test,t1_innodb,id: 5
 DROP,InnoDB,1,test,t1_innodb,id: 5,,0,,,
 CREATE,MyISAM,1,test,t20,id: 6,,0,,,
 DROP,MyISAM,1,test,t20,id: 6,,0,,,

sql/backup.cc

@@ -671,7 +671,10 @@ void backup_log_ddl(const backup_log_info *info)
  ptr= add_name_to_buffer(ptr, &info->org_table);
  ptr= add_id_to_buffer(ptr, &info->org_table_id);
 
- /* The following fields are only set in case of rename */
+ /*
+ The following fields are only set in case of rename, repair or
+ alter table
+ */
  ptr= add_str_to_buffer(ptr, &info->new_storage_engine_name);
  ptr= add_bool_to_buffer(ptr, info->new_partitioned);
  ptr= add_name_to_buffer(ptr, &info->new_database);

sql/handler.h

@@ -2359,12 +2359,14 @@ struct HA_CREATE_INFO: public Table_scope_and_contents_source_st,
 {
  /* TODO: remove after MDEV-20865 */
  Alter_info *alter_info;
+ bool repair;
 
  void init()
  {
  Table_scope_and_contents_source_st::init();
  Schema_specification_st::init();
  alter_info= NULL;
+ repair= 0;
  }
  ulong table_options_with_row_type()
  {

sql/sql_admin.cc

@@ -817,6 +817,25 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
  /* purecov: end */
  }
 
+ /*
+ This has to be tested separately from the following test as
+ optimizer table takes a MDL_SHARED_WRITE lock but we want to
+ log this to the ddl log.
+ */
+
+ if (lock_type == TL_WRITE && table->mdl_request.type >= MDL_SHARED_WRITE)
+ {
+ /* Store information about table for ddl log */
+ storage_engine_partitioned= table->table->file->partition_engine();
+ strmake(storage_engine_name, table->table->file->real_table_type(),
+ sizeof(storage_engine_name)-1);
+ tabledef_version.str= tabledef_version_buff;
+ if ((tabledef_version.length= table->table->s->tabledef_version.length))
+ memcpy((char*) tabledef_version.str,
+ table->table->s->tabledef_version.str,
+ MY_UUID_SIZE);
+ }
+
  /*
  Close all instances of the table to allow MyISAM "repair"
  (which is internally also used from "optimize") to rename files.
@@ -835,16 +854,6 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
  thd->close_unused_temporary_table_instances(table);
  else
  {
- /* Store information about table for ddl log */
- storage_engine_partitioned= table->table->file->partition_engine();
- strmake(storage_engine_name, table->table->file->real_table_type(),
- sizeof(storage_engine_name)-1);
- tabledef_version.str= tabledef_version_buff;
- if ((tabledef_version.length= table->table->s->tabledef_version.length))
- memcpy((char*) tabledef_version.str,
- table->table->s->tabledef_version.str,
- MY_UUID_SIZE);
-
  if (wait_while_table_is_used(thd, table->table, HA_EXTRA_NOT_USED))
  goto err;
  DEBUG_SYNC(thd, "after_admin_flush");
@@ -1476,6 +1485,14 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
  ddl_log.org_database= table->db;
  ddl_log.org_table= table->table_name;
  ddl_log.org_table_id= tabledef_version;
+ if (recreate_used)
+ {
+ LEX_CUSTRING tabledef_version=
+ { recreate_info.tabledef_version, MY_UUID_SIZE };
+ ddl_log.new_database= table->db;
+ ddl_log.new_table= table->table_name;
+ ddl_log.new_table_id= tabledef_version;
+ }
  backup_log_ddl(&ddl_log);
  }
 

sql/sql_class.h

@@ -268,23 +268,22 @@ class CSET_STRING
 
 class Recreate_info
 {
- ha_rows m_records_copied;
- ha_rows m_records_duplicate;
 public:
+ ha_rows copied;
+ ha_rows duplicate;
+ uchar tabledef_version[MY_UUID_SIZE];
+
  Recreate_info()
- :m_records_copied(0),
- m_records_duplicate(0)
- { }
- Recreate_info(ha_rows records_copied,
- ha_rows records_duplicate)
- :m_records_copied(records_copied),
- m_records_duplicate(records_duplicate)
- { }
- ha_rows records_copied() const { return m_records_copied; }
- ha_rows records_duplicate() const { return m_records_duplicate; }
+ :copied(0),
+ duplicate(0)
+ {
+ bzero(tabledef_version, sizeof(tabledef_version));
+ }
+ ha_rows records_copied() const { return copied; }
+ ha_rows records_duplicate() const { return duplicate; }
  ha_rows records_processed() const
  {
- return m_records_copied + m_records_duplicate;
+ return copied + duplicate;
  }
 };
 

sql/sql_table.cc

@@ -12236,7 +12236,11 @@ do_continue:;
  inplace_alter_table_committed= 0;
  }
 
- if (!alter_ctx.tmp_table)
+ /*
+ We do not log ALTER ENTRY, when used with REPAIR, to the ddl.log as the
+ logging will done in mysql_admin_table()
+ */
+ if (!alter_ctx.tmp_table && !create_info->repair)
  {
  backup_log_info ddl_log;
  bzero(&ddl_log, sizeof(ddl_log));
@@ -12253,6 +12257,9 @@ do_continue:;
  ddl_log.new_table_id= alter_ctx.tmp_id;
  backup_log_ddl(&ddl_log);
  }
+ if (create_info->repair)
+ memcpy(recreate_info->tabledef_version, alter_ctx.tmp_id.str,
+ sizeof(recreate_info->tabledef_version));
 
  table_list->table= NULL;// For query cache
  query_cache_invalidate3(thd, table_list, false);
@@ -12272,7 +12279,8 @@ do_continue:;
  thd->variables.option_bits&= ~OPTION_BIN_COMMIT_OFF;
 
  thd_progress_end(thd);
- *recreate_info= Recreate_info(copied, deleted);
+ recreate_info->copied= copied;
+ recreate_info->duplicate= deleted;
  thd->my_ok_with_recreate_info(*recreate_info,
  (ulong) thd->get_stmt_da()->
  current_statement_warn_count());
@@ -13061,6 +13069,7 @@ bool mysql_recreate_table(THD *thd, TABLE_LIST *table_list,
  create_info.row_type=ROW_TYPE_NOT_USED;
  create_info.alter_info= &alter_info;
  create_info.recreate_identical_table= 1;
+ create_info.repair= 1; // Don't write ALTER TABLE to ddl log
  /* Force alter table to recreate table */
  alter_info.flags= (ALTER_CHANGE_COLUMN | ALTER_RECREATE);
  alter_info.partition_flags= thd->lex->alter_info.partition_flags;