diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index bb6af084f38..ace834a26c2 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -3486,12 +3486,13 @@ ALTER TABLE m1 ADD INDEX (c1); UNLOCK TABLES; DROP TABLE m1, t1; # -# Locking the merge table will implicitly lock children. +# Locking the merge table won't implicitly lock children. # CREATE TABLE t1 (c1 INT); CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1); LOCK TABLE m1 WRITE; ALTER TABLE t1 ADD INDEX (c1); +ERROR HY000: Table 't1' was locked with a READ lock and can't be updated LOCK TABLE m1 WRITE, t1 WRITE; ALTER TABLE t1 ADD INDEX (c1); UNLOCK TABLES; @@ -3661,4 +3662,16 @@ REPAIR TABLE t2 USE_FRM; Table Op Msg_type Msg_text test.t2 repair note The storage engine for the table doesn't support repair DROP TABLE t1, t2; +# +# Bug#57002 Assert in upgrade_shared_lock_to_exclusive() +# for ALTER TABLE + MERGE tables +# +DROP TABLE IF EXISTS t1, m1; +CREATE TABLE t1(a INT) engine=myisam; +CREATE TABLE m1(a INT) engine=merge UNION(t1); +LOCK TABLES t1 READ, m1 WRITE; +ALTER TABLE t1 engine=myisam; +ERROR HY000: Table 't1' was locked with a READ lock and can't be updated +UNLOCK TABLES; +DROP TABLE m1, t1; End of 6.0 tests diff --git a/mysql-test/t/merge.test b/mysql-test/t/merge.test index 34d0b5bf237..07839257a15 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -2600,11 +2600,12 @@ UNLOCK TABLES; DROP TABLE m1, t1; --echo # ---echo # Locking the merge table will implicitly lock children. +--echo # Locking the merge table won't implicitly lock children. --echo # CREATE TABLE t1 (c1 INT); CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1); LOCK TABLE m1 WRITE; +--error ER_TABLE_NOT_LOCKED_FOR_WRITE ALTER TABLE t1 ADD INDEX (c1); LOCK TABLE m1 WRITE, t1 WRITE; ALTER TABLE t1 ADD INDEX (c1); @@ -2776,6 +2777,27 @@ REPAIR TABLE t2 USE_FRM; DROP TABLE t1, t2; +--echo # +--echo # Bug#57002 Assert in upgrade_shared_lock_to_exclusive() +--echo # for ALTER TABLE + MERGE tables +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1, m1; +--enable_warnings + +CREATE TABLE t1(a INT) engine=myisam; +CREATE TABLE m1(a INT) engine=merge UNION(t1); +LOCK TABLES t1 READ, m1 WRITE; + +# This caused an assert +--error ER_TABLE_NOT_LOCKED_FOR_WRITE +ALTER TABLE t1 engine=myisam; + +UNLOCK TABLES; +DROP TABLE m1, t1; + + --echo End of 6.0 tests --disable_result_log diff --git a/storage/myisammrg/ha_myisammrg.cc b/storage/myisammrg/ha_myisammrg.cc index 466c66ec769..26e54d61101 100644 --- a/storage/myisammrg/ha_myisammrg.cc +++ b/storage/myisammrg/ha_myisammrg.cc @@ -478,8 +478,31 @@ int ha_myisammrg::add_children_list(void) /* Set the expected table version, to not cause spurious re-prepare. */ child_l->set_table_ref_id(mrg_child_def->get_child_table_ref_type(), mrg_child_def->get_child_def_version()); - /* Use the same metadata lock type for children. */ - child_l->mdl_request.set_type(parent_l->mdl_request.type); + /* + For statements which acquire a SNW metadata lock on a parent table and + then later try to upgrade it to an X lock (e.g. ALTER TABLE), SNW + locks should be also taken on the children tables. + + Otherwise we end up in a situation where the thread trying to upgrade SNW + to X lock on the parent also holds a SR metadata lock and a read + thr_lock.c lock on the child. As a result, another thread might be + blocked on the thr_lock.c lock for the child after successfully acquiring + a SR or SW metadata lock on it. If at the same time this second thread + has a shared metadata lock on the parent table or there is some other + thread which has a shared metadata lock on the parent and is waiting for + this second thread, we get a deadlock. This deadlock cannot be properly + detected by the MDL subsystem as part of the waiting happens within + thr_lock.c. By taking SNW locks on the child tables we ensure that any + thread which waits for a thread doing SNW -> X upgrade, does this within + the MDL subsystem and thus potential deadlocks are exposed to the deadlock + detector. + + We don't do the same thing for SNRW locks as this would allow + DDL on implicitly locked underlying tables of a MERGE table. + */ + if (! thd->locked_tables_mode && + parent_l->mdl_request.type == MDL_SHARED_NO_WRITE) + child_l->mdl_request.set_type(MDL_SHARED_NO_WRITE); /* Link TABLE_LIST object into the children list. */ if (this->children_last_l) child_l->prev_global= this->children_last_l;