From 2543be69423b8a6582a4d2fa688d969359f8de3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 23 Jan 2025 14:38:35 +0200 Subject: [PATCH] MDEV-35854: Clarify row_rename_table_for_mysql() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit enum rename_fk: Replaces the "bool use_fk" parameter of row_rename_table_for_mysql() and innobase_rename_table(): RENAME_IGNORE_FK: Replaces use_fk=false when the operation cannot involve any FOREIGN KEY constraints, that is, it is a partitioned table or an internal table for FULLTEXT INDEX. RENAME_REBUILD: Replaces use_fk=false when the table may contain FOREIGN KEY constraints, which must not be modified in the data dictionary tables SYS_FOREIGN and SYS_FOREIGN_COLS. RENAME_ALTER_COPY: Replaces use_fk=true. This is only specified in ha_innobase::rename_table(), which may be invoked as part of ALTER TABLE…ALGORITHM=COPY, but also during RENAME TABLE. An alternative value RENAME_FK could be useful to specify in ha_innobase::rename_table() when it is executed as part of CREATE OR REPLACE TABLE, which currently is not an atomic operation. Reviewed by: Debarun Banerjee --- storage/innobase/fts/fts0fts.cc | 4 ++-- storage/innobase/handler/ha_innodb.cc | 15 +++++++++------ storage/innobase/handler/handler0alter.cc | 6 ++++-- storage/innobase/include/row0mysql.h | 11 ++++++++++- storage/innobase/row/row0mysql.cc | 15 ++++++++------- 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/storage/innobase/fts/fts0fts.cc b/storage/innobase/fts/fts0fts.cc index df2cb568d4df5..83ae2827230e7 100644 --- a/storage/innobase/fts/fts0fts.cc +++ b/storage/innobase/fts/fts0fts.cc @@ -1392,7 +1392,7 @@ static dberr_t fts_drop_table(trx_t *trx, const char *table_name, bool rename) char *tmp= dict_mem_create_temporary_tablename(heap, table->name.m_name, table->id); dberr_t err= row_rename_table_for_mysql(table->name.m_name, tmp, trx, - false); + RENAME_IGNORE_FK); mem_heap_free(heap); if (err != DB_SUCCESS) { @@ -1450,7 +1450,7 @@ fts_rename_one_aux_table( fts_table_new_name[table_new_name_len] = 0; return row_rename_table_for_mysql( - fts_table_old_name, fts_table_new_name, trx, false); + fts_table_old_name, fts_table_new_name, trx, RENAME_IGNORE_FK); } /****************************************************************//** diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 3229cb672413b..4dd7a9405dd7f 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -13889,10 +13889,10 @@ int ha_innobase::delete_table(const char *name) @param[in,out] trx InnoDB data dictionary transaction @param[in] from old table name @param[in] to new table name -@param[in] use_fk whether to enforce FOREIGN KEY +@param[in] fk how to handle FOREIGN KEY @return DB_SUCCESS or error code */ static dberr_t innobase_rename_table(trx_t *trx, const char *from, - const char *to, bool use_fk) + const char *to, rename_fk fk) { dberr_t error; char norm_to[FN_REFLEN]; @@ -13910,7 +13910,7 @@ static dberr_t innobase_rename_table(trx_t *trx, const char *from, ut_ad(trx->will_lock); - error = row_rename_table_for_mysql(norm_from, norm_to, trx, use_fk); + error = row_rename_table_for_mysql(norm_from, norm_to, trx, fk); if (error != DB_SUCCESS) { if (error == DB_TABLE_NOT_FOUND @@ -13936,7 +13936,8 @@ static dberr_t innobase_rename_table(trx_t *trx, const char *from, #endif /* _WIN32 */ trx_start_if_not_started(trx, true); error = row_rename_table_for_mysql( - par_case_name, norm_to, trx, false); + par_case_name, norm_to, trx, + RENAME_IGNORE_FK); } } @@ -14132,7 +14133,8 @@ int ha_innobase::truncate() if (error == DB_SUCCESS) { - error= innobase_rename_table(trx, ib_table->name.m_name, temp_name, false); + error= innobase_rename_table(trx, ib_table->name.m_name, temp_name, + RENAME_REBUILD); if (error == DB_SUCCESS) error= trx->drop_table(*ib_table); } @@ -14330,7 +14332,8 @@ ha_innobase::rename_table( row_mysql_lock_data_dictionary(trx); if (error == DB_SUCCESS) { - error = innobase_rename_table(trx, from, to, true); + error = innobase_rename_table(trx, from, to, + RENAME_ALTER_COPY); } DEBUG_SYNC(thd, "after_innobase_rename_table"); diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index afcff2f3ea13f..a967259a2abc2 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -10414,10 +10414,12 @@ commit_try_rebuild( char* old_name= mem_heap_strdup(ctx->heap, user_table->name.m_name); dberr_t error = row_rename_table_for_mysql(user_table->name.m_name, - ctx->tmp_name, trx, false); + ctx->tmp_name, trx, + RENAME_REBUILD); if (error == DB_SUCCESS) { error = row_rename_table_for_mysql( - rebuilt_table->name.m_name, old_name, trx, false); + rebuilt_table->name.m_name, old_name, trx, + RENAME_REBUILD); if (error == DB_SUCCESS) { /* The statistics for the surviving indexes will be re-inserted in alter_stats_rebuild(). */ diff --git a/storage/innobase/include/row0mysql.h b/storage/innobase/include/row0mysql.h index 8cbeed7d29706..63858f25f023e 100644 --- a/storage/innobase/include/row0mysql.h +++ b/storage/innobase/include/row0mysql.h @@ -370,6 +370,15 @@ row_import_tablespace_for_mysql( row_prebuilt_t* prebuilt) /*!< in: prebuilt struct in MySQL */ MY_ATTRIBUTE((nonnull, warn_unused_result)); +enum rename_fk { + /** ignore FOREIGN KEY constraints */ + RENAME_IGNORE_FK= 0, + /** Rename a table as part of a native table-rebuilding DDL operation */ + RENAME_REBUILD, + /** Rename as part of ALTER TABLE...ALGORITHM=COPY */ + RENAME_ALTER_COPY +}; + /*********************************************************************//** Renames a table for MySQL. @return error code or DB_SUCCESS */ @@ -379,7 +388,7 @@ row_rename_table_for_mysql( const char* old_name, /*!< in: old table name */ const char* new_name, /*!< in: new table name */ trx_t* trx, /*!< in/out: transaction */ - bool use_fk) /*!< in: whether to parse and enforce + rename_fk fk) /*!< in: how to handle FOREIGN KEY constraints */ MY_ATTRIBUTE((nonnull, warn_unused_result)); diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index ac11ea56ad056..c72e71bf0474e 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -2554,7 +2554,7 @@ row_rename_table_for_mysql( const char* old_name, /*!< in: old table name */ const char* new_name, /*!< in: new table name */ trx_t* trx, /*!< in/out: transaction */ - bool use_fk) /*!< in: whether to parse and enforce + rename_fk fk) /*!< in: how to handle FOREIGN KEY constraints */ { dict_table_t* table = NULL; @@ -2579,6 +2579,8 @@ row_rename_table_for_mysql( old_is_tmp = dict_table_t::is_temporary_name(old_name); new_is_tmp = dict_table_t::is_temporary_name(new_name); + ut_ad(fk != RENAME_IGNORE_FK || !new_is_tmp); + table = dict_table_open_on_name(old_name, true, DICT_ERR_IGNORE_FK_NOKEY); @@ -2638,10 +2640,9 @@ row_rename_table_for_mysql( << TROUBLESHOOTING_MSG; goto funct_exit; - - } else if (use_fk && !old_is_tmp && new_is_tmp) { - /* MySQL is doing an ALTER TABLE command and it renames the - original table to a temporary table name. We want to preserve + } else if (fk == RENAME_ALTER_COPY && !old_is_tmp && new_is_tmp) { + /* Non-native ALTER TABLE is renaming the + original table to a temporary name. We want to preserve the original foreign key constraint definitions despite the name change. An exception is those constraints for which the ALTER TABLE contained DROP FOREIGN KEY .*/ @@ -2685,7 +2686,7 @@ row_rename_table_for_mysql( goto rollback_and_exit; } - if (!new_is_tmp) { + if (/* fk == RENAME_IGNORE_FK || */ !new_is_tmp) { /* Rename all constraints. */ char new_table_name[MAX_TABLE_NAME_LEN + 1]; char old_table_utf8[MAX_TABLE_NAME_LEN + 1]; @@ -2859,7 +2860,7 @@ row_rename_table_for_mysql( err = dict_load_foreigns( new_name, nullptr, trx->id, !old_is_tmp || trx->check_foreigns, - use_fk + fk == RENAME_ALTER_COPY ? DICT_ERR_IGNORE_NONE : DICT_ERR_IGNORE_FK_NOKEY, fk_tables);