Fix hash index delete

kuzudb · Nov 1, 2024 · 16917f5 · 16917f5
1 parent ef0d0b9
commit 16917f5
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 11 deletions.
diff --git a/src/include/storage/index/in_mem_hash_index.h b/src/include/storage/index/in_mem_hash_index.h
@@ -159,7 +159,7 @@ class InMemHashIndex final {
         auto fingerprint = HashIndexUtils::getFingerprintForHash(hashValue);
         auto slotId = HashIndexUtils::getPrimarySlotIdForHash(this->indexHeader, hashValue);
         SlotIterator iter(slotId, this);
-        std::optional<entry_pos_t> deletedPos = 0;
+        std::optional<entry_pos_t> deletedPos;
         do {
             for (auto entryPos = 0u; entryPos < getSlotCapacity<T>(); entryPos++) {
                 if (iter.slot->header.isEntryValid(entryPos) &&
@@ -180,14 +180,16 @@ class InMemHashIndex final {
             auto newIter = iter;
             while (nextChainedSlot(newIter))
                 ;
-            if (newIter.slotInfo != iter.slotInfo ||
-                *deletedPos != newIter.slot->header.numEntries() - 1) {
-                auto lastEntryPos = newIter.slot->header.numEntries();
+            if (newIter.slot->header.numEntries() > 0 &&
+                (newIter.slotInfo != iter.slotInfo ||
+                    *deletedPos != newIter.slot->header.numEntries() - 1)) {
+                auto lastEntryPos = newIter.slot->header.numEntries() - 1;
                 iter.slot->entries[*deletedPos] = newIter.slot->entries[lastEntryPos];
                 iter.slot->header.setEntryValid(*deletedPos,
                     newIter.slot->header.fingerprints[lastEntryPos]);
                 newIter.slot->header.setEntryInvalid(lastEntryPos);
             }
+            return true;
         }
         return false;
     }

diff --git a/src/storage/store/node_group.cpp b/src/storage/store/node_group.cpp
@@ -305,7 +305,7 @@ void NodeGroup::commitInsert(row_idx_t startRow, row_idx_t numRows_,
 std::unique_ptr<ChunkedNodeGroup> NodeGroup::scanAll(MemoryManager& memoryManager,
     const std::vector<common::column_id_t>& columnIDs, const std::vector<const Column*>& columns) {
     auto lock = chunkedGroups.lock();
-    return scanAllInsertedAndVersions<ResidencyState::IN_MEMORY>(memoryManager, lock, columnIDs,
+    return scanAllInsertedAndVersions<ResidencyState::ON_DISK>(memoryManager, lock, columnIDs,
         columns);
 }
 

diff --git a/src/storage/store/node_table.cpp b/src/storage/store/node_table.cpp
@@ -421,7 +421,10 @@ static void indexRollback(ChunkedNodeGroup* chunkedGroup, common::row_idx_t star
         for (row_idx_t i = startRow; i < std::min(startRow + numRows_, chunkedGroup->getNumRows());
              ++i) {
             T key = pkColumnChunk.getValue<T>(i);
-            pkIndex->delete_(key);
+            // TODO fix
+            if (HashIndexUtils::getHashIndexPosition(key) == 0) {
+                pkIndex->delete_(key);
+            }
         }
     };
     TypeUtils::visit(pkColumnChunk.getDataType(), std::cref(rollbackFunc),
@@ -453,7 +456,6 @@ void NodeTable::rollbackInsert(common::row_idx_t startRow, common::row_idx_t num
             auto* nodeGroup = nodeGroups->getNodeGroup(i);
             // scanState->initState(&DUMMY_TRANSACTION, nodeGroup);
             if (curRow >= startRow && curRow + nodeGroup->getNumRows() <= startRow + numRows_) {
-
                 auto startRowInChunk =
                     (startRow < curRow + nodeGroup->getNumRows()) ? (startRow - curRow) : 0;
                 auto numRowsInChunk = nodeGroup->getNumRows() - startRowInChunk;

diff --git a/test/storage/local_hash_index_test.cpp b/test/storage/local_hash_index_test.cpp
@@ -17,17 +17,17 @@ TEST(LocalHashIndexTests, LocalInserts) {
     auto hashIndex =
         std::make_unique<LocalHashIndex>(PhysicalTypeID::INT64, overflowFileHandle.get());
 
-    for (int64_t i = 0u; i < 100; i++) {
+    for (int64_t i = 0u; i < 100000; i++) {
         ASSERT_TRUE(hashIndex->insert(i, i * 2, isVisible));
     }
-    for (int64_t i = 0u; i < 100; i++) {
+    for (int64_t i = 0u; i < 100000; i++) {
         ASSERT_FALSE(hashIndex->insert(i, i, isVisible));
     }
 
-    for (int64_t i = 0u; i < 100; i++) {
+    for (int64_t i = 0u; i < 100000; i++) {
         hashIndex->delete_(i);
     }
-    for (int64_t i = 0u; i < 100; i++) {
+    for (int64_t i = 0u; i < 100000; i++) {
         ASSERT_TRUE(hashIndex->insert(i, i, isVisible));
     }
 }

diff --git a/test/test_files/copy/copy_after_error.test b/test/test_files/copy/copy_after_error.test
@@ -13,6 +13,20 @@ Copy exception: Error in file ${KUZU_ROOT_DIRECTORY}/dataset/ldbc-1/csv/comment_
 ---- 1
 92.39.58.88
 
+-CASE CopyNodeAfterPKErrorRollbackFlushedGroups
+-STATEMENT create node table Comment (id int64, creationDate INT64, locationIP STRING, browserUsed STRING, content STRING, length INT32, PRIMARY KEY (id));
+---- ok
+# COPY will trigger duplicate PK once the 2nd file is hit
+-STATEMENT COPY Comment FROM ['${KUZU_ROOT_DIRECTORY}/dataset/ldbc-sf01/Comment.csv', '${KUZU_ROOT_DIRECTORY}/dataset/ldbc-sf01/Comment.csv'] (delim="|", header=true, parallel=false);
+---- error(regex)
+Copy exception: Found duplicated primary key value \w+, which violates the uniqueness constraint of the primary key column.
+# The failed COPY should revert all of its insertions and the 2nd COPY should succeed
+-STATEMENT COPY Comment FROM '${KUZU_ROOT_DIRECTORY}/dataset/ldbc-sf01/Comment.csv' (DELIM="|", header=true);
+---- ok
+-STATEMENT MATCH (c:Comment) WHERE c.ID = 962073046352 RETURN c.locationIP
+---- 1
+36.95.74.186
+
 -CASE CopyRelAfterNodeCopyError
 -STATEMENT CREATE NODE TABLE Comment(ID INT64, creationDate TIMESTAMP, locationIP STRING, browserUsed STRING, content STRING, length INT64, PRIMARY KEY(ID));
 ---- ok