From: Date: December 23 2008 1:44pm Subject: bzr commit into mysql-6.0-falcon-team branch (svoj:2946) Bug#34182 List-Archive: http://lists.mysql.com/commits/62258 X-Bug: 34182 Message-Id: <20081223124401.7DD8E41CED0@june.myoffice.izhnet.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7BIT #At file:///home/svoj/devel/bzr-mysql/mysql-6.0-falcon-team-bug34182/ 2946 Sergey Vojtovich 2008-12-23 BUG#34182 - SELECT ... FOR UPDATE does not lock when in subquery Fixed that SELECT ... FOR UPDATE/UPDATE/DELETE statements may release previously set record locks for records that do not match current WHERE condition. renamed: mysql-test/suite/falcon_team/r/falcon_deadlock.result => mysql-test/suite/falcon/r/falcon_deadlock.result mysql-test/suite/falcon_team/t/falcon_deadlock.test => mysql-test/suite/falcon/t/falcon_deadlock.test modified: mysql-test/suite/falcon_team/t/test2bug.def storage/falcon/RecordVersion.cpp storage/falcon/RecordVersion.h storage/falcon/Table.cpp storage/falcon/ha_falcon.cpp mysql-test/suite/falcon/r/falcon_deadlock.result mysql-test/suite/falcon/t/falcon_deadlock.test per-file messages: mysql-test/suite/falcon/r/falcon_deadlock.result Fixed falcon_deadlock.test and moved it to falcon suite. mysql-test/suite/falcon/t/falcon_deadlock.test Fixed falcon_deadlock.test and moved it to falcon suite. mysql-test/suite/falcon_team/t/test2bug.def Moved falcon_deadlock.test to falcon suite. storage/falcon/RecordVersion.cpp Initialize "fresh" to true. storage/falcon/RecordVersion.h Added "fresh" member to RecordVersion class. storage/falcon/Table.cpp If a record was already locked by current transaction, inform Table::unlockRecord() about this. In this case we do not release record lock if the record doesn't much provided condition. storage/falcon/ha_falcon.cpp Added a comment. === renamed file 'mysql-test/suite/falcon_team/r/falcon_deadlock.result' => 'mysql-test/suite/falcon/r/falcon_deadlock.result' --- a/mysql-test/suite/falcon_team/r/falcon_deadlock.result 2008-12-18 10:49:29 +0000 +++ b/mysql-test/suite/falcon/r/falcon_deadlock.result 2008-12-23 12:43:42 +0000 @@ -26,6 +26,7 @@ id x 0 1 COMMIT; # Switch to connection conn2 +ERROR HY000: Record has changed since last read in table 't1' ROLLBACK; # Switch to connection conn1 SELECT * FROM t1; @@ -50,10 +51,10 @@ a b 0 10 1 20 2 30 -UPDATE t2 SET a = 100 WHERE b = (SELECT x FROM t1 WHERE id = b FOR UPDATE); +UPDATE t2 SET b = 100 WHERE a = (SELECT x FROM t1 WHERE id = a FOR UPDATE); SELECT * FROM t2; a b -0 10 +0 100 1 20 2 30 SELECT * FROM t1; @@ -70,11 +71,12 @@ id x 300 300 COMMIT; # Switch to connection conn2 +ERROR HY000: Record has changed since last read in table 't1' COMMIT; # Switch to connection conn1 SELECT * FROM t1; id x -0 2 +0 1 300 300 COMMIT; DROP TABLE t1; @@ -121,11 +123,12 @@ id x 300 300 COMMIT; # Switch to connection conn2 +ERROR HY000: Record has changed since last read in table 't1' COMMIT; # Switch to connection conn1 SELECT * FROM t1; id x -0 2 +0 1 300 300 COMMIT; SELECT count(*) FROM t1; === renamed file 'mysql-test/suite/falcon_team/t/falcon_deadlock.test' => 'mysql-test/suite/falcon/t/falcon_deadlock.test' --- a/mysql-test/suite/falcon_team/t/falcon_deadlock.test 2008-12-18 10:49:29 +0000 +++ b/mysql-test/suite/falcon/t/falcon_deadlock.test 2008-12-23 12:43:42 +0000 @@ -98,7 +98,7 @@ INSERT INTO t2 VALUES (0, 10), (1, 20), COMMIT; SELECT * FROM t2; -UPDATE t2 SET a = 100 WHERE b = (SELECT x FROM t1 WHERE id = b FOR UPDATE); +UPDATE t2 SET b = 100 WHERE a = (SELECT x FROM t1 WHERE id = a FOR UPDATE); SELECT * FROM t2; SELECT * FROM t1; @@ -117,6 +117,14 @@ COMMIT; --echo # Switch to connection conn2 connection conn2; +# If we set Falcon to +# falcon_consistent_read = off +# +# we should get a +# Record has changed since last read in table 't1' +# here. Please note that falcon_consistent_read = off +# is the default setting. +--error ER_CHECKREAD --reap COMMIT; @@ -166,6 +174,14 @@ COMMIT; --echo # Switch to connection conn2 connection conn2; +# If we set Falcon to +# falcon_consistent_read = off +# +# we should get a +# Record has changed since last read in table 't1' +# here. Please note that falcon_consistent_read = off +# is the default setting. +--error ER_CHECKREAD --reap COMMIT; === modified file 'mysql-test/suite/falcon_team/t/test2bug.def' --- a/mysql-test/suite/falcon_team/t/test2bug.def 2008-12-16 21:11:25 +0000 +++ b/mysql-test/suite/falcon_team/t/test2bug.def 2008-12-23 12:43:42 +0000 @@ -19,4 +19,3 @@ falcon_bug_34174 : Bug#34174 - Infinite loop checking rolled back record in select for update falcon_bug_36294-big: Bug#36631 - Assertion in SerialLogControl::nextRecord, sometimes. -falcon_deadlock: Bug#34182 - SELECT ... FOR UPDATE does not lock when in subquery === modified file 'storage/falcon/RecordVersion.cpp' --- a/storage/falcon/RecordVersion.cpp 2008-11-07 01:09:04 +0000 +++ b/storage/falcon/RecordVersion.cpp 2008-12-23 12:43:42 +0000 @@ -50,6 +50,7 @@ RecordVersion::RecordVersion(Table *tbl, transactionId = transaction->transactionId; savePointId = transaction->curSavePointId; superceded = false; + fresh = true; if ((priorVersion = oldVersion)) { @@ -72,6 +73,7 @@ RecordVersion::RecordVersion(Database* d transactionId = stream->getInt(); int priorType = stream->getInt(); superceded = false; + fresh = true; if (priorType == 0) priorVersion = new Record(database, stream); === modified file 'storage/falcon/RecordVersion.h' --- a/storage/falcon/RecordVersion.h 2008-10-29 23:25:13 +0000 +++ b/storage/falcon/RecordVersion.h 2008-12-23 12:43:42 +0000 @@ -72,6 +72,7 @@ public: TransId transactionId; int savePointId; bool superceded; + bool fresh; }; #endif // !defined(AFX_RECORDVERSION_H__84FD1965_A97F_11D2_AB5C_0000C01D2301__INCLUDED_) === modified file 'storage/falcon/Table.cpp' --- a/storage/falcon/Table.cpp 2008-12-16 20:40:38 +0000 +++ b/storage/falcon/Table.cpp 2008-12-23 12:43:42 +0000 @@ -3445,7 +3445,7 @@ void Table::unlockRecord(int recordNumbe if (record) { - if (record->state == recLock) + if (record->state == recLock && ((RecordVersion*) record)->fresh) unlockRecord((RecordVersion*) record); record->release(); @@ -3491,6 +3491,12 @@ void Table::checkAncestor(Record* curren // Unlike the sister routine, fetchVersion, this function will release // the refCount on source if nothing is returned. But since this // does not have a catch, no functions below it should throw an exception. +// +// It may happen that the candidate was already locked for update by +// this transaction, meaning we don't have to lock it once again. +// +// As no lock occured here, we must inform Table::unlockRecord() +// about it by setting RecordVersion::fresh to false. Record* Table::fetchForUpdate(Transaction* transaction, Record* source, bool usingIndex) { @@ -3516,6 +3522,7 @@ Record* Table::fetchForUpdate(Transactio Record *prior = record->getPriorVersion(); prior->addRef(); + ((RecordVersion*) record)->fresh = false; record->release(); return prior; === modified file 'storage/falcon/ha_falcon.cpp' --- a/storage/falcon/ha_falcon.cpp 2008-12-18 01:44:03 +0000 +++ b/storage/falcon/ha_falcon.cpp 2008-12-23 12:43:42 +0000 @@ -944,6 +944,22 @@ bool StorageInterface::check_if_incompat return COMPATIBLE_DATA_YES; } +// How does SELECT ... FOR UPDATE work: +// The server is upgrading lock type for SELECT ... FOR UPDATE, +// so here we get lock_type set to TL_WRITE. +// +// Falcon remembers that we got a write lock and sets lockForUpdate +// accordingly. Note that we may get TL_WRITE with UPDATE query, +// which may also lock the row during table scan, and we must still +// lock the row in this case. +// +// When performing table/index scan underlying functions are expected +// to lock fetched rows. It may happen that retrieved row doesn't match +// a condition. In this case server calls StorageInterface::unlock_row(), +// which is intended to release just locked row. +// +// Note that the row may be locked by previous table/index scans. In this +// case ::unlock_row() must preserve the lock. THR_LOCK_DATA **StorageInterface::store_lock(THD *thd, THR_LOCK_DATA **to, enum thr_lock_type lock_type) {