Author: kahatlen
Date: Mon Mar 16 12:43:21 2009
New Revision: 754881
URL: http://svn.apache.org/viewvc?rev=754881&view=rev
Log:
DERBY-4028: two rows can be inserted with the same value in a column that a unique constraint
on that column should prevent
Merged fix from trunk (revision 752826).
Modified:
db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/store/access/btree/BTreeController.java
db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/NullableUniqueConstraintTest.java
Modified: db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/store/access/btree/BTreeController.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/store/access/btree/BTreeController.java?rev=754881&r1=754880&r2=754881&view=diff
==============================================================================
--- db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/store/access/btree/BTreeController.java
(original)
+++ db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/store/access/btree/BTreeController.java
Mon Mar 16 12:43:21 2009
@@ -404,16 +404,37 @@
rh = leaf.page.fetchFromSlot(null, slot, rows, null, true);
if (rh != null) {
int ret = compareRowsForInsert(rows, oldRows, leaf, slot);
- //release the page if required
- if (ret == RESCAN_REQUIRED && newLeaf) {
- originalLeaf.release();
- }
- if (ret != RESCAN_REQUIRED && newLeaf) {
- leaf.release();
+
+ // If we found a deleted row, we don't know whether there
+ // is a duplicate, so we need to continue the search.
+ final boolean continueSearch =
+ (ret == MATCH_FOUND && leaf.page.isDeletedAtSlot(slot));
+
+ if (!continueSearch) {
+ if (newLeaf) {
+ // Since we have moved away from the original leaf,
+ // we need some logic to make sure we don't hold
+ // latches that we're not supposed to hold.
+ if (ret == RESCAN_REQUIRED) {
+ // When a rescan is required, we must release the
+ // original leaf, since the callers expect all
+ // latches to have been released (and so they
+ // should have been, so this is probably a bug -
+ // see DERBY-4080).
+ originalLeaf.release();
+ }
+ if (ret != RESCAN_REQUIRED) {
+ // Since a rescan is not required, we still hold
+ // the latch on the non-original leaf. No other
+ // leaves than the original one should be latched
+ // when we return, so release the current leaf.
+ leaf.release();
+ }
+ }
+ return ret;
}
- return ret;
}
- slot++;
+ slot--;
}
return NO_MATCH;
}
@@ -464,13 +485,35 @@
rh = leaf.page.fetchFromSlot(null, slot, rows, null, true);
if (rh != null) {
int ret = compareRowsForInsert(rows, oldRows, leaf, slot);
- if (ret == RESCAN_REQUIRED && newLeaf) {
- originalLeaf.release();
- }
- if (ret != RESCAN_REQUIRED && newLeaf) {
- leaf.release();
+
+ // If we found a deleted row, we don't know whether there
+ // is a duplicate, so we need to continue the search.
+ final boolean continueSearch =
+ (ret == MATCH_FOUND && leaf.page.isDeletedAtSlot(slot));
+
+ if (!continueSearch) {
+ if (newLeaf) {
+ // Since we have moved away from the original leaf,
+ // we need some logic to make sure we don't hold
+ // latches that we're not supposed to hold.
+ if (ret == RESCAN_REQUIRED) {
+ // When a rescan is required, we must release the
+ // original leaf, since the callers expect all
+ // latches to have been released (and so they
+ // should have been, so this is probably a bug -
+ // see DERBY-4080).
+ originalLeaf.release();
+ }
+ if (ret != RESCAN_REQUIRED) {
+ // Since a rescan is not required, we still hold
+ // the latch on the non-original leaf. No other
+ // leaves than the original one should be latched
+ // when we return, so release the current leaf.
+ leaf.release();
+ }
+ }
+ return ret;
}
- return ret;
}
slot++;
}
@@ -478,22 +521,31 @@
}
/**
- * Compares two row for insert. If the two rows are equal it checks if the
- * row in tree is deleted. If not MATCH_FOUND is returned. If the row is
- * deleted it tries to get a lock on that. If a lock is obtained without
- * waiting (ie without losing the latch) the row was deleted within the
- * same transaction and its safe to insert. NO_MATCH is returned in this
- * case. If latch is released while waiting for lock rescaning the tree
- * is required as the tree might have been rearanged by some other
- * transaction. RESCAN_REQUIRED is returned in this case.
- * In case of NO_MATCH and MATCH_FOUND latch is also released.
+ * Compares two rows for insert. If the two rows are not equal,
+ * {@link #NO_MATCH} is returned. Otherwise, it tries to get a lock on
+ * the row in the tree. If the lock is obtained without waiting,
+ * {@link #MATCH_FOUND} is returned (even if the row has been deleted).
+ * Otherwise, {@link #RESCAN_REQUIRED} is returned to indicate that the
+ * latches have been released and the B-tree must be rescanned.
+ *
+ * If {@code MATCH_FOUND} is returned, the caller should check whether
+ * the row has been deleted. If so, it may have to move to check the
+ * adjacent rows to be sure that there is no non-deleted duplicate row.
+ *
+ * If {@code MATCH_FOUND} or {@code RESCAN_REQUIRED} is returned, the
+ * transaction will hold an update lock on the specified record when
+ * the method returns.
+ *
+ * <b>Note!</b> This method should only be called when the index is almost
+ * unique (that is, a non-unique index backing a unique constraint).
+ *
* @param originalRow row from the tree
* @param newRow row to be inserted
* @param leaf leaf where originalRow resides
* @param slot slot where originalRow
- * @return 0 if no duplicate
- * 1 if duplicate
- * 2 if rescan required
+ * @return {@code NO_MATCH} if no duplicate is found,
+ * {@code MATCH_FOUND} if a duplicate is found, or
+ * {@code RESCAN_REQUIRED} if the B-tree must be rescanned
*/
private int compareRowsForInsert (DataValueDescriptor [] originalRow,
DataValueDescriptor [] newRow,
@@ -516,14 +568,8 @@
//record and might have changed the tree by now
if (latch_released)
return RESCAN_REQUIRED;
- //there is match check if its not deleted
- if (!leaf.page.isDeletedAtSlot(slot)) {
- //its a genuine match
- return MATCH_FOUND;
- }
- //it is a deleted record within same transaction
- //safe to insert
- return NO_MATCH;
+
+ return MATCH_FOUND;
}
/**
Modified: db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/NullableUniqueConstraintTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/NullableUniqueConstraintTest.java?rev=754881&r1=754880&r2=754881&view=diff
==============================================================================
--- db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/NullableUniqueConstraintTest.java
(original)
+++ db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/lang/NullableUniqueConstraintTest.java
Mon Mar 16 12:43:21 2009
@@ -483,6 +483,50 @@
}
}
+ /**
+ * Test that a deleted duplicate value on the right side of the slot
+ * into which a new value is inserted does not hide a non-deleted
+ * duplicate two slots to the right. DERBY-4028
+ */
+ public void testDeletedDuplicateHidesDuplicateOnRightSide()
+ throws SQLException {
+ Statement s = createStatement();
+ s.execute("alter table constraintest add constraint c unique(val1)");
+ s.execute("insert into constraintest(val1) values '1','2','3'");
+ // Make sure there's a deleted index entry for val1 = 2
+ s.execute("delete from constraintest where val1 = '2'");
+ // Make sure there's an index entry for val1 = 2 after the deleted one
+ // (the third row will be located after the deleted one because it
+ // was inserted later and its record id is greater)
+ s.execute("update constraintest set val1 = '2' where val1 = '3'");
+ // Insert an index entry in front of the deleted one. It should fail,
+ // but before DERBY-4028 it was successfully inserted.
+ assertStatementError("23505", s,
+ "update constraintest set val1 = '2' where val1 = '1'");
+ }
+
+ /**
+ * Test that a deleted duplicate value on the left side of the slot
+ * into which a new value is inserted does not hide a non-deleted
+ * duplicate two slots to the left. DERBY-4028
+ */
+ public void testDeletedDuplicateHidesDuplicateOnLeftSide()
+ throws SQLException {
+ Statement s = createStatement();
+ s.execute("alter table constraintest add constraint c unique(val1)");
+ s.execute("insert into constraintest(val1) values '1','2','3'");
+ // Make sure there's a deleted index entry for val1 = 2
+ s.execute("delete from constraintest where val1 = '2'");
+ // Make sure there's an index entry for val1 = 2 in front of the
+ // deleted one (the first row will be located in front of the deleted
+ // one because it was inserted before and its record id is smaller)
+ s.execute("update constraintest set val1 = '2' where val1 = '1'");
+ // Insert an index entry after the deleted one. It should fail,
+ // but before DERBY-4028 it was successfully inserted.
+ assertStatementError("23505", s,
+ "update constraintest set val1 = '2' where val1 = '3'");
+ }
+
public static void main(String [] args) {
TestResult tr = new TestResult();
Test t = suite();
|