diff --git a/mysql-test/suite/innodb/r/foreign_key.result b/mysql-test/suite/innodb/r/foreign_key.result index 808e2270e27..80a0afb8d06 100644 --- a/mysql-test/suite/innodb/r/foreign_key.result +++ b/mysql-test/suite/innodb/r/foreign_key.result @@ -1035,9 +1035,22 @@ BEGIN; INSERT INTO child SET a=1; ERROR 23000: Cannot add or update a child row: a foreign key constraint fails (`test`.`child`, CONSTRAINT `child_ibfk_1` FOREIGN KEY (`a`) REFERENCES `parent` (`a`)) connection default; +TRUNCATE TABLE parent; +ERROR 42000: Cannot truncate a table referenced in a foreign key constraint (`test`.`child`, CONSTRAINT `child_ibfk_1` FOREIGN KEY (`a`) REFERENCES `test`.`parent` (`a`)) +DROP TABLE parent; +ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails +SET innodb_lock_wait_timeout=0; +RENAME TABLE parent TO transparent; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +ALTER TABLE parent FORCE, ALGORITHM=COPY; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +ALTER TABLE parent FORCE, ALGORITHM=INPLACE; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction SET innodb_lock_wait_timeout=0, foreign_key_checks=0; TRUNCATE TABLE parent; ERROR HY000: Lock wait timeout exceeded; try restarting transaction +DROP TABLE parent; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction ALTER TABLE parent FORCE, ALGORITHM=COPY; ERROR HY000: Lock wait timeout exceeded; try restarting transaction ALTER TABLE parent FORCE, ALGORITHM=INPLACE; @@ -1052,7 +1065,13 @@ TRUNCATE TABLE parent; ALTER TABLE parent FORCE, ALGORITHM=COPY; ALTER TABLE parent FORCE, ALGORITHM=INPLACE; ALTER TABLE parent ADD COLUMN b INT, ALGORITHM=INSTANT; -DROP TABLE child, parent; +SET foreign_key_checks=ON; +TRUNCATE TABLE parent; +ERROR 42000: Cannot truncate a table referenced in a foreign key constraint (`test`.`child`, CONSTRAINT `child_ibfk_1` FOREIGN KEY (`a`) REFERENCES `test`.`parent` (`a`)) +ALTER TABLE parent FORCE, ALGORITHM=COPY; +ALTER TABLE parent FORCE, ALGORITHM=INPLACE; +RENAME TABLE parent TO transparent; +DROP TABLE child, transparent; # # MDEV-26217 Failing assertion: list.count > 0 in ut_list_remove # or Assertion `lock->trx == this' failed in dberr_t trx_t::drop_table diff --git a/mysql-test/suite/innodb/t/foreign_key.test b/mysql-test/suite/innodb/t/foreign_key.test index 0db3a7ca377..e793e261abd 100644 --- a/mysql-test/suite/innodb/t/foreign_key.test +++ b/mysql-test/suite/innodb/t/foreign_key.test @@ -1077,10 +1077,23 @@ BEGIN; --error ER_NO_REFERENCED_ROW_2 INSERT INTO child SET a=1; connection default; +--error ER_TRUNCATE_ILLEGAL_FK +TRUNCATE TABLE parent; +--error ER_ROW_IS_REFERENCED_2 +DROP TABLE parent; +SET innodb_lock_wait_timeout=0; +--error ER_LOCK_WAIT_TIMEOUT +RENAME TABLE parent TO transparent; +--error ER_LOCK_WAIT_TIMEOUT +ALTER TABLE parent FORCE, ALGORITHM=COPY; +--error ER_LOCK_WAIT_TIMEOUT +ALTER TABLE parent FORCE, ALGORITHM=INPLACE; SET innodb_lock_wait_timeout=0, foreign_key_checks=0; --error ER_LOCK_WAIT_TIMEOUT TRUNCATE TABLE parent; --error ER_LOCK_WAIT_TIMEOUT +DROP TABLE parent; +--error ER_LOCK_WAIT_TIMEOUT ALTER TABLE parent FORCE, ALGORITHM=COPY; --error ER_LOCK_WAIT_TIMEOUT ALTER TABLE parent FORCE, ALGORITHM=INPLACE; @@ -1095,7 +1108,13 @@ TRUNCATE TABLE parent; ALTER TABLE parent FORCE, ALGORITHM=COPY; ALTER TABLE parent FORCE, ALGORITHM=INPLACE; ALTER TABLE parent ADD COLUMN b INT, ALGORITHM=INSTANT; -DROP TABLE child, parent; +SET foreign_key_checks=ON; +--error ER_TRUNCATE_ILLEGAL_FK +TRUNCATE TABLE parent; +ALTER TABLE parent FORCE, ALGORITHM=COPY; +ALTER TABLE parent FORCE, ALGORITHM=INPLACE; +RENAME TABLE parent TO transparent; +DROP TABLE child, transparent; --echo # --echo # MDEV-26217 Failing assertion: list.count > 0 in ut_list_remove diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index 623c2c3dd7a..7f4f73d969b 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -657,47 +657,22 @@ dict_table_t::parse_name<>(char(&)[NAME_LEN + 1], char(&)[NAME_LEN + 1], /** Acquire MDL shared for the table name. @tparam trylock whether to use non-blocking operation @param[in,out] table table object -@param[in,out] thd background thread -@param[out] mdl mdl ticket +@param[in,out] mdl_context MDL context +@param[out] mdl MDL ticket @param[in] table_op operation to perform when opening @return table object after locking MDL shared @retval nullptr if the table is not readable, or if trylock && MDL blocked */ template +__attribute__((nonnull, warn_unused_result)) dict_table_t* dict_acquire_mdl_shared(dict_table_t *table, - THD *thd, - MDL_ticket **mdl, + MDL_context *mdl_context, MDL_ticket **mdl, dict_table_op_t table_op) { - if (!table || !mdl) - return table; - - MDL_context *mdl_context= static_cast(thd_mdl_context(thd)); - size_t db_len; - dict_table_t *not_found= nullptr; - - if (trylock) - { - dict_sys.freeze(SRW_LOCK_CALL); - db_len= dict_get_db_name_len(table->name.m_name); - dict_sys.unfreeze(); - } - else - { - ut_ad(dict_sys.frozen_not_locked()); - db_len= dict_get_db_name_len(table->name.m_name); - } - - if (db_len == 0) - return table; /* InnoDB system tables are not covered by MDL */ - - if (!mdl_context) - return nullptr; - table_id_t table_id= table->id; char db_buf[NAME_LEN + 1], db_buf1[NAME_LEN + 1]; char tbl_buf[NAME_LEN + 1], tbl_buf1[NAME_LEN + 1]; - size_t tbl_len; + size_t db_len, tbl_len; bool unaccessible= false; if (!table->parse_name(db_buf, tbl_buf, &db_len, &tbl_len)) @@ -768,7 +743,6 @@ retry: if (!table || !table->is_accessible()) { - table= nullptr; return_without_mdl: if (trylock) dict_sys.unfreeze(); @@ -777,7 +751,7 @@ return_without_mdl: mdl_context->release_lock(*mdl); *mdl= nullptr; } - return not_found; + return nullptr; } size_t db1_len, tbl1_len; @@ -814,6 +788,50 @@ return_without_mdl: goto retry; } +template dict_table_t* dict_acquire_mdl_shared +(dict_table_t*,MDL_context*,MDL_ticket**,dict_table_op_t); + +/** Acquire MDL shared for the table name. +@tparam trylock whether to use non-blocking operation +@param[in,out] table table object +@param[in,out] thd background thread +@param[out] mdl mdl ticket +@param[in] table_op operation to perform when opening +@return table object after locking MDL shared +@retval nullptr if the table is not readable, or if trylock && MDL blocked */ +template +dict_table_t* +dict_acquire_mdl_shared(dict_table_t *table, + THD *thd, + MDL_ticket **mdl, + dict_table_op_t table_op) +{ + if (!table || !mdl) + return table; + + MDL_context *mdl_context= static_cast(thd_mdl_context(thd)); + size_t db_len; + + if (trylock) + { + dict_sys.freeze(SRW_LOCK_CALL); + db_len= dict_get_db_name_len(table->name.m_name); + dict_sys.unfreeze(); + } + else + { + ut_ad(dict_sys.frozen_not_locked()); + db_len= dict_get_db_name_len(table->name.m_name); + } + + if (db_len == 0) + return table; /* InnoDB system tables are not covered by MDL */ + + return mdl_context + ? dict_acquire_mdl_shared(table, mdl_context, mdl, table_op) + : nullptr; +} + template dict_table_t* dict_acquire_mdl_shared (dict_table_t*,THD*,MDL_ticket**,dict_table_op_t); template dict_table_t* dict_acquire_mdl_shared diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 92f8a60ecf8..1cf6fa80ace 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -13395,6 +13395,49 @@ ha_innobase::discard_or_import_tablespace( DBUG_RETURN(0); } +/** Report a DROP TABLE failure due to a FOREIGN KEY constraint. +@param name table name +@param foreign constraint */ +ATTRIBUTE_COLD +static void delete_table_cannot_drop_foreign(const table_name_t &name, + const dict_foreign_t &foreign) +{ + mysql_mutex_lock(&dict_foreign_err_mutex); + rewind(dict_foreign_err_file); + ut_print_timestamp(dict_foreign_err_file); + fputs(" Cannot drop table ", dict_foreign_err_file); + ut_print_name(dict_foreign_err_file, nullptr, name.m_name); + fputs("\nbecause it is referenced by ", dict_foreign_err_file); + ut_print_name(dict_foreign_err_file, nullptr, foreign.foreign_table_name); + putc('\n', dict_foreign_err_file); + mysql_mutex_unlock(&dict_foreign_err_mutex); +} + +/** Check if DROP TABLE would fail due to a FOREIGN KEY constraint. +@param table table to be dropped +@param sqlcom thd_sql_command(current_thd) +@return whether child tables that refer to this table exist */ +static bool delete_table_check_foreigns(const dict_table_t &table, + enum_sql_command sqlcom) +{ + const bool drop_db{sqlcom == SQLCOM_DROP_DB}; + for (const auto foreign : table.referenced_set) + { + /* We should allow dropping a referenced table if creating + that referenced table has failed for some reason. For example + if referenced table is created but it column types that are + referenced do not match. */ + if (foreign->foreign_table == &table || + (drop_db && + dict_tables_have_same_db(table.name.m_name, + foreign->foreign_table_name_lookup))) + continue; + delete_table_cannot_drop_foreign(table.name, *foreign); + return true; + } + + return false; +} /** DROP TABLE (possibly as part of DROP DATABASE, CREATE/ALTER TABLE) @param name table name @@ -13410,6 +13453,7 @@ int ha_innobase::delete_table(const char *name) DBUG_EXECUTE_IF("test_normalize_table_name_low", test_normalize_table_name_low();); + const enum_sql_command sqlcom= enum_sql_command(thd_sql_command(thd)); trx_t *parent_trx= check_trx_exists(thd); dict_table_t *table; @@ -13446,6 +13490,13 @@ int ha_innobase::delete_table(const char *name) DBUG_RETURN(0); } + if (parent_trx->check_foreigns && + delete_table_check_foreigns(*table, sqlcom)) + { + dict_sys.unlock(); + DBUG_RETURN(HA_ERR_ROW_IS_REFERENCED); + } + table->acquire(); dict_sys.unlock(); @@ -13478,14 +13529,7 @@ int ha_innobase::delete_table(const char *name) /* FOREIGN KEY constraints cannot exist on partitioned tables. */; #endif else - { - dict_sys.freeze(SRW_LOCK_CALL); - for (const dict_foreign_t* f : table->referenced_set) - if (dict_table_t* child= f->foreign_table) - if ((err= lock_table_for_trx(child, trx, LOCK_X)) != DB_SUCCESS) - break; - dict_sys.unfreeze(); - } + err= lock_table_children(table, trx); } dict_table_t *table_stats= nullptr, *index_stats= nullptr; @@ -13495,7 +13539,6 @@ int ha_innobase::delete_table(const char *name) const bool fts= err == DB_SUCCESS && (table->flags2 & (DICT_TF2_FTS_HAS_DOC_ID | DICT_TF2_FTS)); - const enum_sql_command sqlcom= enum_sql_command(thd_sql_command(thd)); if (fts) { @@ -13653,36 +13696,16 @@ err_exit: DBUG_RETURN(convert_error_code_to_mysql(err, 0, NULL)); } - if (!table->no_rollback() && trx->check_foreigns) + if (!table->no_rollback()) { - const bool drop_db= sqlcom == SQLCOM_DROP_DB; - for (auto foreign : table->referenced_set) + if (trx->check_foreigns && delete_table_check_foreigns(*table, sqlcom)) { - /* We should allow dropping a referenced table if creating - that referenced table has failed for some reason. For example - if referenced table is created but it column types that are - referenced do not match. */ - if (foreign->foreign_table == table || - (drop_db && - dict_tables_have_same_db(table->name.m_name, - foreign->foreign_table_name_lookup))) - continue; - mysql_mutex_lock(&dict_foreign_err_mutex); - rewind(dict_foreign_err_file); - ut_print_timestamp(dict_foreign_err_file); - fputs(" Cannot drop table ", dict_foreign_err_file); - ut_print_name(dict_foreign_err_file, trx, table->name.m_name); - fputs("\nbecause it is referenced by ", dict_foreign_err_file); - ut_print_name(dict_foreign_err_file, trx, foreign->foreign_table_name); - putc('\n', dict_foreign_err_file); - mysql_mutex_unlock(&dict_foreign_err_mutex); err= DB_CANNOT_DROP_CONSTRAINT; goto err_exit; } - } - if (!table->no_rollback()) err= trx->drop_table_foreign(table->name); + } if (err == DB_SUCCESS && table_stats && index_stats) err= trx->drop_table_statistics(table->name); @@ -13801,6 +13824,19 @@ int ha_innobase::truncate() update_thd(); +#ifdef UNIV_DEBUG + if (!thd_test_options(m_user_thd, OPTION_NO_FOREIGN_KEY_CHECKS)) + { + /* fk_truncate_illegal_if_parent() should have failed in + Sql_cmd_truncate_table::handler_truncate() if foreign_key_checks=ON + and child tables exist. */ + dict_sys.freeze(SRW_LOCK_CALL); + for (const auto foreign : m_prebuilt->table->referenced_set) + ut_ad(foreign->foreign_table == m_prebuilt->table); + dict_sys.unfreeze(); + } +#endif + if (is_read_only()) DBUG_RETURN(HA_ERR_TABLE_READONLY); @@ -13883,14 +13919,7 @@ int ha_innobase::truncate() dict_table_t *table_stats = nullptr, *index_stats = nullptr; MDL_ticket *mdl_table = nullptr, *mdl_index = nullptr; - dberr_t error= DB_SUCCESS; - - dict_sys.freeze(SRW_LOCK_CALL); - for (const dict_foreign_t *f : ib_table->referenced_set) - if (dict_table_t *child= f->foreign_table) - if ((error= lock_table_for_trx(child, trx, LOCK_X)) != DB_SUCCESS) - break; - dict_sys.unfreeze(); + dberr_t error= lock_table_children(ib_table, trx); if (error == DB_SUCCESS) error= lock_table_for_trx(ib_table, trx, LOCK_X); @@ -14081,16 +14110,7 @@ ha_innobase::rename_table( /* There is no need to lock any FOREIGN KEY child tables. */ } else if (dict_table_t *table = dict_table_open_on_name( norm_from, false, DICT_ERR_IGNORE_FK_NOKEY)) { - dict_sys.freeze(SRW_LOCK_CALL); - for (const dict_foreign_t* f : table->referenced_set) { - if (dict_table_t* child = f->foreign_table) { - error = lock_table_for_trx(child, trx, LOCK_X); - if (error != DB_SUCCESS) { - break; - } - } - } - dict_sys.unfreeze(); + error = lock_table_children(table, trx); if (error == DB_SUCCESS) { error = lock_table_for_trx(table, trx, LOCK_X); } diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 3b892434a67..dc5c1f71925 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -11203,16 +11203,7 @@ ha_innobase::commit_inplace_alter_table( fts_optimize_remove_table(ctx->old_table); } - dict_sys.freeze(SRW_LOCK_CALL); - for (auto f : ctx->old_table->referenced_set) { - if (dict_table_t* child = f->foreign_table) { - error = lock_table_for_trx(child, trx, LOCK_X); - if (error != DB_SUCCESS) { - break; - } - } - } - dict_sys.unfreeze(); + error = lock_table_children(ctx->old_table, trx); if (ctx->new_table->fts) { ut_ad(!ctx->new_table->fts->add_wq); diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h index 5b45f55f7c6..6f9a0efe02c 100644 --- a/storage/innobase/include/dict0dict.h +++ b/storage/innobase/include/dict0dict.h @@ -35,6 +35,7 @@ Created 1/8/1996 Heikki Tuuri #include #include +class MDL_context; class MDL_ticket; /** the first table or index ID for other than hard-coded system tables */ @@ -139,6 +140,21 @@ dict_acquire_mdl_shared(dict_table_t *table, MDL_ticket **mdl, dict_table_op_t table_op= DICT_TABLE_OP_NORMAL); +/** Acquire MDL shared for the table name. +@tparam trylock whether to use non-blocking operation +@param[in,out] table table object +@param[in,out] mdl_context MDL context +@param[out] mdl MDL ticket +@param[in] table_op operation to perform when opening +@return table object after locking MDL shared +@retval nullptr if the table is not readable, or if trylock && MDL blocked */ +template +__attribute__((nonnull, warn_unused_result)) +dict_table_t* +dict_acquire_mdl_shared(dict_table_t *table, + MDL_context *mdl_context, MDL_ticket **mdl, + dict_table_op_t table_op); + /** Look up a table by numeric identifier. @param[in] table_id table identifier @param[in] dict_locked data dictionary locked diff --git a/storage/innobase/include/lock0lock.h b/storage/innobase/include/lock0lock.h index e7b8e839c12..08b9f4bcb35 100644 --- a/storage/innobase/include/lock0lock.h +++ b/storage/innobase/include/lock0lock.h @@ -438,6 +438,13 @@ dberr_t lock_table_for_trx(dict_table_t *table, trx_t *trx, lock_mode mode, bool no_wait= false) MY_ATTRIBUTE((nonnull, warn_unused_result)); +/** Lock the child tables of a table. +@param table parent table +@param trx transaction +@return error code */ +dberr_t lock_table_children(dict_table_t *table, trx_t *trx) + MY_ATTRIBUTE((nonnull, warn_unused_result)); + /** Exclusively lock the data dictionary tables. @param trx dictionary transaction @return error code diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 6b1ca9d753f..907eaf58997 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -47,6 +47,7 @@ Created 5/7/1996 Heikki Tuuri #include "que0que.h" #include "scope.h" #include +#include #include @@ -3933,6 +3934,8 @@ static void lock_table_dequeue(lock_t *in_lock, bool owns_wait_mutex) dberr_t lock_table_for_trx(dict_table_t *table, trx_t *trx, lock_mode mode, bool no_wait) { + ut_ad(!dict_sys.frozen()); + mem_heap_t *heap= mem_heap_create(512); sel_node_t *node= sel_node_create(heap); que_thr_t *thr= pars_complete_graph_for_exec(node, trx, heap, nullptr); @@ -3969,6 +3972,67 @@ run_again: return err; } +/** Lock the child tables of a table. +@param table parent table +@param trx transaction +@return error code */ +dberr_t lock_table_children(dict_table_t *table, trx_t *trx) +{ + MDL_context *mdl_context= + static_cast(thd_mdl_context(trx->mysql_thd)); + ut_ad(mdl_context); + struct table_mdl{dict_table_t* table; MDL_ticket *mdl;}; + std::vector children; + children.emplace_back(table_mdl{table, nullptr}); + + dberr_t err= DB_SUCCESS; + dict_sys.freeze(SRW_LOCK_CALL); + + rescan: + for (auto f : table->referenced_set) + if (dict_table_t *child= f->foreign_table) + { + if (std::find_if(children.begin(), children.end(), + [&](const table_mdl &c){ return c.table == child; }) != + children.end()) + continue; /* We already acquired MDL on this child table. */ + MDL_ticket *mdl= nullptr; + child->acquire(); + child= dict_acquire_mdl_shared(child, mdl_context, &mdl, + DICT_TABLE_OP_NORMAL); + if (child) + { + if (!mdl) + child->release(); + children.emplace_back(table_mdl{child, mdl}); + goto rescan; + } + err= DB_LOCK_WAIT_TIMEOUT; + break; + } + dict_sys.unfreeze(); + + if (err == DB_SUCCESS) + for (const table_mdl &child : children) + if (child.mdl) + if ((err= lock_table_for_trx(child.table, trx, LOCK_X)) != DB_SUCCESS) + break; + + dict_sys.freeze(SRW_LOCK_CALL); + for (table_mdl &child : children) + { + if (child.mdl) + { + child.table->release(); + mdl_context->release_lock(child.mdl); + } + } + dict_sys.unfreeze(); + + return err; +} + + /** Exclusively lock the data dictionary tables. @param trx dictionary transaction @return error code