db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kahat...@apache.org
Subject svn commit: r754881 - in /db/derby/code/branches/10.4/java: engine/org/apache/derby/impl/store/access/btree/BTreeController.java testing/org/apache/derbyTesting/functionTests/tests/lang/NullableUniqueConstraintTest.java
Date Mon, 16 Mar 2009 12:43:22 GMT
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();



Mime
View raw message