Commit 149b7547 authored by Sergey Vojtovich's avatar Sergey Vojtovich

MDEV-17595 - ALTER TABLE ADD FOREIGN KEY crash

ALTER TABLE ... ADD FOREIGN KEY may trigger assertion failure when
it has LOCK=EXCLUSIVE clause or concurrent FLUSH TABLES is being
executed.

In both cases being altered table is marked as flushed, which forces
subsequent attempt to open parent table to re-open. Which in turn is
not allowed while transaction is running.

Rather than opening parent table, just take appropriate MDL lock.

Also removed table_already_fk_prelocked() check: MDL itself has much
better methods to handle duplicate locks. E.g. the former won't acquire
MDL_SHARED_NO_WRITE if it already has MDL_SHARED_READ.
parent 0a0eed80
...@@ -87,3 +87,16 @@ drop table t3; ...@@ -87,3 +87,16 @@ drop table t3;
drop table t2; drop table t2;
drop table t1; drop table t1;
set debug_sync='reset'; set debug_sync='reset';
#
# MDEV-17595 - Server crashes in copy_data_between_tables or
# Assertion `thd->transaction.stmt.is_empty() ||
# (thd->state_flags & Open_tables_state::BACKUPS_AVAIL)'
# fails in close_tables_for_reopen upon concurrent
# ALTER TABLE and FLUSH
#
CREATE TABLE t1 (a INT, KEY(a)) ENGINE=InnoDB;
INSERT INTO t1 VALUES(1),(2);
CREATE TABLE t2 (b INT, KEY(b)) ENGINE=InnoDB;
INSERT INTO t2 VALUES(2);
ALTER TABLE t2 ADD FOREIGN KEY(b) REFERENCES t1(a), LOCK=EXCLUSIVE;
DROP TABLE t2, t1;
...@@ -111,3 +111,18 @@ drop table t3; ...@@ -111,3 +111,18 @@ drop table t3;
drop table t2; drop table t2;
drop table t1; drop table t1;
set debug_sync='reset'; set debug_sync='reset';
--echo #
--echo # MDEV-17595 - Server crashes in copy_data_between_tables or
--echo # Assertion `thd->transaction.stmt.is_empty() ||
--echo # (thd->state_flags & Open_tables_state::BACKUPS_AVAIL)'
--echo # fails in close_tables_for_reopen upon concurrent
--echo # ALTER TABLE and FLUSH
--echo #
CREATE TABLE t1 (a INT, KEY(a)) ENGINE=InnoDB;
INSERT INTO t1 VALUES(1),(2);
CREATE TABLE t2 (b INT, KEY(b)) ENGINE=InnoDB;
INSERT INTO t2 VALUES(2);
ALTER TABLE t2 ADD FOREIGN KEY(b) REFERENCES t1(a), LOCK=EXCLUSIVE;
DROP TABLE t2, t1;
...@@ -9074,6 +9074,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -9074,6 +9074,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
new_table->file->get_foreign_key_list(thd, &fk_list); new_table->file->get_foreign_key_list(thd, &fk_list);
while ((fk= fk_list_it++)) while ((fk= fk_list_it++))
{ {
MDL_request mdl_request;
if (lower_case_table_names) if (lower_case_table_names)
{ {
char buf[NAME_LEN]; char buf[NAME_LEN];
...@@ -9085,22 +9087,16 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -9085,22 +9087,16 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
len = my_casedn_str(files_charset_info, buf); len = my_casedn_str(files_charset_info, buf);
thd->make_lex_string(fk->referenced_table, buf, len); thd->make_lex_string(fk->referenced_table, buf, len);
} }
if (table_already_fk_prelocked(table_list, fk->referenced_db,
fk->referenced_table, TL_READ_NO_INSERT))
continue;
TABLE_LIST *tl= (TABLE_LIST *) thd->alloc(sizeof(TABLE_LIST));
tl->init_one_table_for_prelocking(fk->referenced_db->str, fk->referenced_db->length,
fk->referenced_table->str, fk->referenced_table->length,
NULL, TL_READ_NO_INSERT, false, NULL, 0,
&thd->lex->query_tables_last);
}
if (open_tables(thd, &table_list->next_global, &tables_opened, 0, mdl_request.init(MDL_key::TABLE,
&alter_prelocking_strategy)) fk->referenced_db->str, fk->referenced_table->str,
MDL_SHARED_NO_WRITE, MDL_TRANSACTION);
if (thd->mdl_context.acquire_lock(&mdl_request,
thd->variables.lock_wait_timeout))
goto err_new_table_cleanup; goto err_new_table_cleanup;
} }
} }
}
/* /*
Note: In case of MERGE table, we do not attach children. We do not Note: In case of MERGE table, we do not attach children. We do not
copy data for MERGE tables. Only the children have data. copy data for MERGE tables. Only the children have data.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment