bzr commit into mysql-6.0-falcon-team branch (svoj:2946) Bug#34182

[Sergey Vojtovich]

#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)
 {