bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2712) Bug#43344

[Kevin Lewis]

#At file:///C:/Work/bzr/Merge/mysql-6.0-falcon-team/ based on revid:kevin.lewis@stripped

 2712 Kevin Lewis	2009-05-27
      Bug#43344 - Thsi patch is just like the previous one, except that a bug is fixed.  Instead of checking if (dup->state != recDeleted) it correctly checks if (dup->state == recDeleted).
      
      The assert is that after hasRecord() returns false, the state is neither recLock, recRollback, nor recDeleted.  It is not recChilled anymore since that state is no longer used. 
      
      Since Table::checkUniqueRecordVersion() can be called for any record along a prior record chain at almost any time, it is possible that valid lock or deleted record versions have other temporary states such as recNoChill, recDeleting & recEndChain.  So this assertion is not valid and should be deleted.  The code after "getRelativeState(dup, DO_NOT_WAIT)" and "if (!dup->hasRecord()" should be changed to only check the state for a deleted record if the relative state is Us or CommittedVisible.

    modified:
      storage/falcon/Table.cpp
=== modified file 'storage/falcon/Table.cpp'
--- a/storage/falcon/Table.cpp	2009-05-20 16:58:14 +0000
+++ b/storage/falcon/Table.cpp	2009-05-27 19:32:23 +0000
@@ -2577,48 +2577,53 @@ bool Table::checkUniqueRecordVersion(int
 
 		if (!dup->hasRecord())
 			{
-			// If the record is a lock record, keep looking for a dup.
-
-			if (dup->state == recLock)
-				continue;  // Next record version.
-
-			if (dup->state == recRollback)
-				continue;  // Next record version.
-			
-			// The record has been deleted.
-			ASSERT(dup->state == recDeleted);
+			// A record without a data buffer is either a lock record or a deleted record.
+			// Checking the state is tenuous because it can be changed temporarilty
+			// (See recNoChill, recInserting, recDeleting, recEndChain)
+			// So we usualy keep looking for duplicates in older record versions .
 
 			switch (state)
 				{
 				case CommittedVisible:
 				case Us:
 					// No conflict with a visible deleted record.
-					rec->release(REC_HISTORY);
-					
-					if (activeTransState)
-						activeTransState->release();
-					
-					return false;	// Check next record number.
+					// If the state was recDeleted and is now recDeleting, it is being scavenged.
+					// So there is no harm in checking prior.  
+					// The other temporary states are not possible.
+
+					if (dup->state == recDeleted)
+						{
+						rec->release(REC_HISTORY);
+
+						if (activeTransState)
+							activeTransState->release();
+
+						return false;	// Check next record number.
+						}
+					else
+						continue;	// Next record version.
 
 				case CommittedInvisible:
 					// This state only happens for consistent read
+
 					ASSERT(IS_CONSISTENT_READ(transaction->isolationLevel));
-					foundFirstCommitted = true;
+
+					foundFirstCommitted = true;  // skip other committedInvisible
 					
 					continue;	// Next record version.
 
 				case Active:
-					// A pending transaction deleted a record.
 					// Keep looking for a possible duplicate conflict,
 					// either visible, or pending at a savepoint.
+					// We may have to come back and wait on this transaction
 
 					activeTransState = dup->getTransactionState();
 					activeTransState->addRef();
 					
-					continue;
+					continue;	// Next record version.
 
 				default:
-					continue;   // record was deleted, keep looking for a dup.
+					continue;   // Next record version, keep looking for a duplicate.
 				}
 			}
 


Attachment: [text/bzr-bundle] bzr/[e-mail]