diff --git a/mysql-test/suite/innodb/r/rename_table_debug.result b/mysql-test/suite/innodb/r/rename_table_debug.result index d13fc84aa43..48211ad0aca 100644 --- a/mysql-test/suite/innodb/r/rename_table_debug.result +++ b/mysql-test/suite/innodb/r/rename_table_debug.result @@ -11,4 +11,21 @@ disconnect con1; SELECT * FROM t1; a b c d 1 NULL NULL NULL -DROP TABLE t1; +CREATE TABLE t2 (a INT PRIMARY KEY) ENGINE=InnoDB; +BEGIN; +INSERT INTO t2 VALUES(1); +connect con1,localhost,root,,test; +SET DEBUG_SYNC='innodb_rename_in_cache SIGNAL committed WAIT_FOR ever'; +RENAME TABLE t1 TO t3; +connection default; +SET DEBUG_SYNC='now WAIT_FOR committed'; +COMMIT; +# restart +disconnect con1; +SELECT * FROM t1; +a b c d +1 NULL NULL NULL +SELECT * FROM t2; +a +1 +DROP TABLE t1,t2; diff --git a/mysql-test/suite/innodb/t/alter_rename_existing.test b/mysql-test/suite/innodb/t/alter_rename_existing.test index 06602ae8e74..556d8e660b4 100644 --- a/mysql-test/suite/innodb/t/alter_rename_existing.test +++ b/mysql-test/suite/innodb/t/alter_rename_existing.test @@ -88,6 +88,6 @@ SELECT * from t1; DROP TABLE t1; --disable_query_log -call mtr.add_suppression("\\[ERROR\\] InnoDB: Cannot rename '.*' to '.*' because the target file exists. Remove the target file and try again"); +call mtr.add_suppression("\\[ERROR\\] InnoDB: Cannot rename '.*' to '.*' because the target file exists"); SET GLOBAL innodb_file_per_table = @old_innodb_file_per_table; --enable_query_log diff --git a/mysql-test/suite/innodb/t/foreign_key.test b/mysql-test/suite/innodb/t/foreign_key.test index be248a5bed4..cb157fc90db 100644 --- a/mysql-test/suite/innodb/t/foreign_key.test +++ b/mysql-test/suite/innodb/t/foreign_key.test @@ -148,9 +148,7 @@ SET FOREIGN_KEY_CHECKS=1; --echo # --disable_query_log -call mtr.add_suppression("InnoDB: Possible reasons:"); -call mtr.add_suppression("InnoDB: \\([12]\\) Table "); -call mtr.add_suppression("InnoDB: If table `test`\\.`t2` is a temporary table"); +call mtr.add_suppression("InnoDB: Table rename might cause two FOREIGN KEY"); call mtr.add_suppression("InnoDB: Cannot delete/update rows with cascading foreign key constraints that exceed max depth of 15\\."); --enable_query_log diff --git a/mysql-test/suite/innodb/t/rename_table_debug.test b/mysql-test/suite/innodb/t/rename_table_debug.test index c75f2fbca10..3e2de242d49 100644 --- a/mysql-test/suite/innodb/t/rename_table_debug.test +++ b/mysql-test/suite/innodb/t/rename_table_debug.test @@ -19,4 +19,23 @@ SET DEBUG_SYNC='now WAIT_FOR renamed'; --source include/restart_mysqld.inc --disconnect con1 SELECT * FROM t1; -DROP TABLE t1; + +CREATE TABLE t2 (a INT PRIMARY KEY) ENGINE=InnoDB; +BEGIN; +INSERT INTO t2 VALUES(1); + +--connect (con1,localhost,root,,test) +SET DEBUG_SYNC='innodb_rename_in_cache SIGNAL committed WAIT_FOR ever'; +--send +RENAME TABLE t1 TO t3; +--connection default +SET DEBUG_SYNC='now WAIT_FOR committed'; +COMMIT; + +--let $shutdown_timeout=0 +--source include/restart_mysqld.inc +--disconnect con1 +SELECT * FROM t1; +SELECT * FROM t2; + +DROP TABLE t1,t2; diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index da8a2bce1ff..3287041ccb2 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -1543,6 +1543,66 @@ struct dict_foreign_remove_partial } }; +/** Rename the data file. +@param new_name name of the table +@param replace whether to replace the file with the new name + (as part of rolling back TRUNCATE) */ +dberr_t +dict_table_t::rename_tablespace(const char *new_name, bool replace) const +{ + ut_ad(dict_table_is_file_per_table(this)); + ut_ad(!is_temporary()); + + if (!space) + { + const char *data_dir= DICT_TF_HAS_DATA_DIR(flags) + ? data_dir_path : nullptr; + ut_ad(data_dir || !DICT_TF_HAS_DATA_DIR(flags)); + + if (char *filepath= fil_make_filepath(data_dir, name, IBD, + data_dir != nullptr)) + { + fil_delete_tablespace(space_id, true); + os_file_type_t ftype; + bool exists; + /* Delete any temp file hanging around. */ + if (os_file_status(filepath, &exists, &ftype) && exists && + !os_file_delete_if_exists(innodb_temp_file_key, filepath, nullptr)) + ib::info() << "Delete of " << filepath << " failed."; + ut_free(filepath); + } + return DB_SUCCESS; + } + + const char *old_path= UT_LIST_GET_FIRST(space->chain)->name; + fil_space_t::name_type space_name{new_name, strlen(new_name)}; + const bool data_dir= DICT_TF_HAS_DATA_DIR(flags); + char *path= data_dir + ? os_file_make_new_pathname(old_path, new_name) + : fil_make_filepath(nullptr, space_name, IBD, false); + dberr_t err; + if (!path) + err= DB_OUT_OF_MEMORY; + else if (!strcmp(path, old_path)) + err= DB_SUCCESS; + else if (data_dir && + DB_SUCCESS != RemoteDatafile::create_link_file(space_name, path)) + err= DB_TABLESPACE_EXISTS; + else + { + err= space->rename(path, true, replace); + if (data_dir) + { + if (err == DB_SUCCESS) + space_name= {name.m_name, strlen(name.m_name)}; + RemoteDatafile::delete_link_file(space_name); + } + } + + ut_free(path); + return err; +} + /**********************************************************************//** Renames a table object. @return TRUE if success */ @@ -1560,11 +1620,9 @@ dict_table_rename_in_cache( file with the new name (as part of rolling back TRUNCATE) */ { - dberr_t err; dict_foreign_t* foreign; ulint fold; char old_name[MAX_FULL_NAME_LEN + 1]; - os_file_type_t ftype; dict_sys.assert_locked(); @@ -1590,88 +1648,10 @@ dict_table_rename_in_cache( return(DB_ERROR); } - /* If the table is stored in a single-table tablespace, rename the - .ibd file and rebuild the .isl file if needed. */ - - if (!table->space) { - bool exists; - char* filepath; - - ut_ad(dict_table_is_file_per_table(table)); - ut_ad(!table->is_temporary()); - - /* Make sure the data_dir_path is set. */ - dict_get_and_save_data_dir_path(table, true); - - const char* data_dir = DICT_TF_HAS_DATA_DIR(table->flags) - ? table->data_dir_path : nullptr; - ut_ad(data_dir || !DICT_TF_HAS_DATA_DIR(table->flags)); - - filepath = fil_make_filepath(data_dir, table->name, IBD, - data_dir != nullptr); - - if (filepath == NULL) { - return(DB_OUT_OF_MEMORY); - } - - fil_delete_tablespace(table->space_id, !table->space); - - /* Delete any temp file hanging around. */ - if (os_file_status(filepath, &exists, &ftype) - && exists - && !os_file_delete_if_exists(innodb_temp_file_key, - filepath, NULL)) { - - ib::info() << "Delete of " << filepath << " failed."; - } - ut_free(filepath); - - } else if (dict_table_is_file_per_table(table)) { - char* new_path; - const char* old_path = UT_LIST_GET_FIRST(table->space->chain) - ->name; - - ut_ad(!table->is_temporary()); - - const fil_space_t::name_type new_space_name{ - new_name, strlen(new_name)}; - - if (DICT_TF_HAS_DATA_DIR(table->flags)) { - new_path = os_file_make_new_pathname( - old_path, new_name); - err = RemoteDatafile::create_link_file( - new_space_name, new_path); - - if (err != DB_SUCCESS) { - ut_free(new_path); - return(DB_TABLESPACE_EXISTS); - } - } else { - new_path = fil_make_filepath( - NULL, new_space_name, IBD, false); - } - - /* New filepath must not exist. */ - err = table->space->rename(new_path, true, replace_new_file); - ut_free(new_path); - - /* If the tablespace is remote, a new .isl file was created - If success, delete the old one. If not, delete the new one. */ - - if (err != DB_SUCCESS) { - if (DICT_TF_HAS_DATA_DIR(table->flags)) { - RemoteDatafile::delete_link_file( - new_space_name); - } - - return err; - } - - if (DICT_TF_HAS_DATA_DIR(table->flags)) { - RemoteDatafile::delete_link_file( - {old_name, strlen(old_name)}); - } - + if (!dict_table_is_file_per_table(table)) { + } else if (dberr_t err = table->rename_tablespace(new_name, + replace_new_file)) { + return err; } /* Remove table from the hash tables of tables */ diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index 947076f9b4a..ef23e036876 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -1867,88 +1867,41 @@ char *fil_make_filepath(const char* path, const table_name_t name, suffix, strip_name); } -/** Test if a tablespace file can be renamed to a new filepath by checking -if that the old filepath exists and the new filepath does not exist. -@param[in] old_path old filepath -@param[in] new_path new filepath -@param[in] replace_new whether to ignore the existence of new_path -@return innodb error code */ -static dberr_t -fil_rename_tablespace_check( - const char* old_path, - const char* new_path, - bool replace_new) +dberr_t fil_space_t::rename(const char *path, bool log, bool replace) { - bool exists = false; - os_file_type_t ftype; + ut_ad(UT_LIST_GET_LEN(chain) == 1); + ut_ad(!is_system_tablespace(id)); - if (os_file_status(old_path, &exists, &ftype) && !exists) { - ib::error() << "Cannot rename '" << old_path - << "' to '" << new_path - << "' because the source file" - << " does not exist."; - return(DB_TABLESPACE_NOT_FOUND); - } + const char *old_path= chain.start->name; - exists = false; - if (os_file_status(new_path, &exists, &ftype) && !exists) { - return DB_SUCCESS; - } + if (!strcmp(path, old_path)) + return DB_SUCCESS; - if (!replace_new) { - ib::error() << "Cannot rename '" << old_path - << "' to '" << new_path - << "' because the target file exists." - " Remove the target file and try again."; - return(DB_TABLESPACE_EXISTS); - } + if (log) + { + bool exists= false; + os_file_type_t ftype; - /* This must be during the ROLLBACK of TRUNCATE TABLE. - Because InnoDB only allows at most one data dictionary - transaction at a time, and because this incomplete TRUNCATE - would have created a new tablespace file, we must remove - a possibly existing tablespace that is associated with the - new tablespace file. */ -retry: - mysql_mutex_lock(&fil_system.mutex); - for (fil_space_t& space : fil_system.space_list) { - ulint id = space.id; - if (id - && space.purpose == FIL_TYPE_TABLESPACE - && !strcmp(new_path, - UT_LIST_GET_FIRST(space.chain)->name)) { - ib::info() << "TRUNCATE rollback: " << id - << "," << new_path; - mysql_mutex_unlock(&fil_system.mutex); - dberr_t err = fil_delete_tablespace(id); - if (err != DB_SUCCESS) { - return err; - } - goto retry; - } - } - mysql_mutex_unlock(&fil_system.mutex); - fil_delete_file(new_path); + if (os_file_status(old_path, &exists, &ftype) && !exists) + { + ib::error() << "Cannot rename '" << old_path << "' to '" << path + << "' because the source file does not exist."; + return DB_TABLESPACE_NOT_FOUND; + } - return(DB_SUCCESS); -} + exists= false; + if (replace); + else if (!os_file_status(path, &exists, &ftype) || exists) + { + ib::error() << "Cannot rename '" << old_path << "' to '" << path + << "' because the target file exists."; + return DB_TABLESPACE_EXISTS; + } -dberr_t fil_space_t::rename(const char* path, bool log, bool replace) -{ - ut_ad(UT_LIST_GET_LEN(chain) == 1); - ut_ad(!is_system_tablespace(id)); + fil_name_write_rename(id, old_path, path); + } - if (log) { - dberr_t err = fil_rename_tablespace_check( - chain.start->name, path, replace); - if (err != DB_SUCCESS) { - return(err); - } - fil_name_write_rename(id, chain.start->name, path); - } - - return fil_rename_tablespace(id, chain.start->name, path) - ? DB_SUCCESS : DB_ERROR; + return fil_rename_tablespace(id, old_path, path) ? DB_SUCCESS : DB_ERROR; } /** Rename a single-table tablespace. diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index a7f04c3015e..22d70f311cb 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -2045,6 +2045,12 @@ struct dict_table_t { void stats_mutex_lock() { lock_mutex_lock(); } void stats_mutex_unlock() { lock_mutex_unlock(); } + /** Rename the data file. + @param new_name name of the table + @param replace whether to replace the file with the new name + (as part of rolling back TRUNCATE) */ + dberr_t rename_tablespace(const char *new_name, bool replace) const; + private: /** Initialize instant->field_map. @param[in] table table definition to copy from */ diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index c2ef867a41e..2e5a15498d8 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -1622,7 +1622,8 @@ os_file_rename_func( /* New path must not exist. */ ut_ad(os_file_status(newpath, &exists, &type)); - ut_ad(!exists); + /* MDEV-25506 FIXME: Remove the strstr() */ + ut_ad(!exists || strstr(oldpath, "/" TEMP_FILE_PREFIX_INNODB)); /* Old path must exist. */ ut_ad(os_file_status(oldpath, &exists, &type)); diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index cf979d03e0a..e557c13e986 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -4093,7 +4093,6 @@ row_rename_table_for_mysql( ibool old_is_tmp, new_is_tmp; pars_info_t* info = NULL; int retry; - bool aux_fts_rename = false; char* is_part = NULL; ut_a(old_name != NULL); @@ -4253,7 +4252,7 @@ row_rename_table_for_mysql( if (err != DB_SUCCESS) { // Assume the caller guarantees destination name doesn't exist. ut_ad(err != DB_DUPLICATE_KEY); - goto err_exit; + goto rollback_and_exit; } if (!new_is_tmp) { @@ -4392,54 +4391,28 @@ row_rename_table_for_mysql( || DICT_TF2_FLAG_IS_SET(table, DICT_TF2_FTS_HAS_DOC_ID)) && !dict_tables_have_same_db(old_name, new_name)) { err = fts_rename_aux_tables(table, new_name, trx); - if (err != DB_TABLE_NOT_FOUND) { - aux_fts_rename = true; - } } - if (err != DB_SUCCESS) { -err_exit: - if (err == DB_DUPLICATE_KEY) { - ib::error() << "Possible reasons:"; - ib::error() << "(1) Table rename would cause two" - " FOREIGN KEY constraints to have the same" - " internal name in case-insensitive" - " comparison."; - ib::error() << "(2) Table " - << ut_get_name(trx, new_name) - << " exists in the InnoDB internal data" - " dictionary though MySQL is trying to rename" - " table " << ut_get_name(trx, old_name) - << " to it. Have you deleted the .frm file and" - " not used DROP TABLE?"; - ib::info() << TROUBLESHOOTING_MSG; - ib::error() << "If table " - << ut_get_name(trx, new_name) - << " is a temporary table #sql..., then" - " it can be that there are still queries" - " running on the table, and it will be dropped" - " automatically when the queries end. You can" - " drop the orphaned table inside InnoDB by" - " creating an InnoDB table with the same name" - " in another database and copying the .frm file" - " to the current database. Then MySQL thinks" - " the table exists, and DROP TABLE will" - " succeed."; - } + switch (err) { + case DB_DUPLICATE_KEY: + ib::error() << "Table rename might cause two" + " FOREIGN KEY constraints to have the same" + " internal name in case-insensitive comparison."; + ib::info() << TROUBLESHOOTING_MSG; + /* fall through */ + rollback_and_exit: + default: trx->error_state = DB_SUCCESS; trx->rollback(); trx->error_state = DB_SUCCESS; - } else { - /* The following call will also rename the .ibd data file if - the table is stored in a single-table tablespace */ - + break; + case DB_SUCCESS: + DEBUG_SYNC_C("innodb_rename_in_cache"); + /* The following call will also rename the .ibd file */ err = dict_table_rename_in_cache( table, new_name, !new_is_tmp); if (err != DB_SUCCESS) { - trx->error_state = DB_SUCCESS; - trx->rollback(); - trx->error_state = DB_SUCCESS; - goto funct_exit; + goto rollback_and_exit; } /* In case of copy alter, template db_name and @@ -4462,7 +4435,6 @@ err_exit: fk_tables); if (err != DB_SUCCESS) { - if (old_is_tmp) { /* In case of copy alter, ignore the loading of foreign key constraint @@ -4476,7 +4448,7 @@ err_exit: " definition."; if (!trx->check_foreigns) { err = DB_SUCCESS; - goto funct_exit; + break; } } else { ib::error() << "In RENAME TABLE table " @@ -4486,22 +4458,14 @@ err_exit: " with the new table definition."; } - trx->error_state = DB_SUCCESS; - trx->rollback(); - trx->error_state = DB_SUCCESS; + goto rollback_and_exit; } /* Check whether virtual column or stored column affects the foreign key constraint of the table. */ - if (dict_foreigns_has_s_base_col( - table->foreign_set, table)) { + if (dict_foreigns_has_s_base_col(table->foreign_set, table)) { err = DB_NO_FK_ON_S_BASE_COL; - ut_a(DB_SUCCESS == dict_table_rename_in_cache( - table, old_name, FALSE)); - trx->error_state = DB_SUCCESS; - trx->rollback(); - trx->error_state = DB_SUCCESS; - goto funct_exit; + goto rollback_and_exit; } /* Fill the virtual column set in foreign when @@ -4519,37 +4483,6 @@ err_exit: } funct_exit: - if (aux_fts_rename && err != DB_SUCCESS - && table != NULL && (table->space != 0)) { - - char* orig_name = table->name.m_name; - trx_t* trx_bg = trx_create(); - - /* If the first fts_rename fails, the trx would - be rolled back and committed, we can't use it any more, - so we have to start a new background trx here. */ - ut_a(trx_state_eq(trx_bg, TRX_STATE_NOT_STARTED)); - trx_bg->op_info = "Revert the failing rename " - "for fts aux tables"; - trx_bg->dict_operation_lock_mode = RW_X_LATCH; - trx_start_for_ddl(trx_bg, TRX_DICT_OP_TABLE); - - /* If rename fails and table has its own tablespace, - we need to call fts_rename_aux_tables again to - revert the ibd file rename, which is not under the - control of trx. Also notice the parent table name - in cache is not changed yet. If the reverting fails, - the ibd data may be left in the new database, which - can be fixed only manually. */ - table->name.m_name = const_cast(new_name); - fts_rename_aux_tables(table, old_name, trx_bg); - table->name.m_name = orig_name; - - trx_bg->dict_operation_lock_mode = 0; - trx_commit_for_mysql(trx_bg); - trx_bg->free(); - } - if (table != NULL) { if (commit && !table->is_temporary()) { table->stats_bg_flag &= byte(~BG_STAT_SHOULD_QUIT); diff --git a/storage/innobase/row/row0uins.cc b/storage/innobase/row/row0uins.cc index e4a4e2f18e3..a549e2fc2b1 100644 --- a/storage/innobase/row/row0uins.cc +++ b/storage/innobase/row/row0uins.cc @@ -398,8 +398,12 @@ static bool row_undo_ins_parse_undo_rec(undo_node_t* node, bool dict_locked) ptr[len] = 0; const char* name = reinterpret_cast(ptr); if (strcmp(table->name.m_name, name)) { - dict_table_rename_in_cache(table, name, false, - table_id != 0); + dict_table_rename_in_cache(table, name, false, true); + } else if (table->space) { + const auto s = table->space->name(); + if (len != s.size() || memcmp(name, s.data(), len)) { + table->rename_tablespace(name, true); + } } goto close_table; }