Merge pull request #1132 from kuzudb/delete-rel-from-col

add delete-rel support for column
kuzudb · Dec 21, 2022 · e8d0da8 · e8d0da8
2 parents 04efdad + 8d9255c
commit e8d0da8
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 5 deletions.
diff --git a/src/storage/store/rel_table.cpp b/src/storage/store/rel_table.cpp
@@ -170,8 +170,7 @@ void RelTable::rollbackInMemoryIfNecessary() {
 void RelTable::insertRel(const shared_ptr<ValueVector>& srcNodeIDVector,
     const shared_ptr<ValueVector>& dstNodeIDVector,
     const vector<shared_ptr<ValueVector>>& relPropertyVectors) {
-    assert(srcNodeIDVector->state->isFlat());
-    assert(dstNodeIDVector->state->isFlat());
+    assert(srcNodeIDVector->state->isFlat() && dstNodeIDVector->state->isFlat());
     auto srcTableID =
         srcNodeIDVector->getValue<nodeID_t>(srcNodeIDVector->state->selVector->selectedPositions[0])
             .tableID;
@@ -208,6 +207,24 @@ void RelTable::deleteRel(const shared_ptr<ValueVector>& srcNodeIDVector,
     const shared_ptr<ValueVector>& dstNodeIDVector, const shared_ptr<ValueVector>& relIDVector) {
     assert(srcNodeIDVector->state->isFlat() && dstNodeIDVector->state->isFlat() &&
            relIDVector->state->isFlat());
+    auto srcTableID =
+        srcNodeIDVector->getValue<nodeID_t>(srcNodeIDVector->state->selVector->selectedPositions[0])
+            .tableID;
+    auto dstTableID =
+        dstNodeIDVector->getValue<nodeID_t>(dstNodeIDVector->state->selVector->selectedPositions[0])
+            .tableID;
+    for (auto direction : REL_DIRECTIONS) {
+        auto boundTableID = (direction == RelDirection::FWD ? srcTableID : dstTableID);
+        auto boundVector = (direction == RelDirection::FWD ? srcNodeIDVector : dstNodeIDVector);
+        if (adjColumns[direction].contains(boundTableID)) {
+            auto nodeOffset =
+                boundVector->readNodeOffset(boundVector->state->selVector->selectedPositions[0]);
+            adjColumns[direction].at(boundTableID)->setNodeOffsetToNull(nodeOffset);
+            for (auto i = 0; i < propertyColumns[direction].size(); i++) {
+                propertyColumns[direction].at(boundTableID)[i]->setNodeOffsetToNull(nodeOffset);
+            }
+        }
+    }
     listsUpdateStore->deleteRelIfNecessary(srcNodeIDVector, dstNodeIDVector, relIDVector);
 }
 

diff --git a/test/runner/e2e_delete_rel_test.cpp b/test/runner/e2e_delete_rel_test.cpp
@@ -13,10 +13,21 @@ class DeleteRelTest : public DBTest {
             srcTable.c_str(), dstTable.c_str(), srcID, dstID);
     }
     string getInsertKnowsRelQuery(string srcTable, string dstTable, int64_t srcID, int64_t dstID) {
-        auto str = StringUtils::string_format("MATCH (p1:%s), (p2:%s) WHERE p1.ID = %d AND p2.ID "
-                                              "= %d create (p1)-[:knows {length: %d}]->(p2);",
+        return StringUtils::string_format("MATCH (p1:%s), (p2:%s) WHERE p1.ID = %d AND p2.ID "
+                                          "= %d create (p1)-[:knows {length: %d}]->(p2);",
             srcTable.c_str(), dstTable.c_str(), srcID, dstID, srcID);
-        return str;
+    }
+
+    string getDeleteTeachesRelQuery(int64_t srcID, int64_t dstID) {
+        return StringUtils::string_format(
+            "MATCH (p1:person)-[t:teaches]->(p2:person) WHERE p1.ID = %d AND p2.ID = %d delete t;",
+            srcID, dstID);
+    }
+
+    string getDeleteHasOwnerRelQuery(int64_t srcID, int64_t dstID) {
+        return StringUtils::string_format(
+            "MATCH (a:animal)-[h:hasOwner]->(p:person) WHERE a.ID = %d AND p.ID = %d delete h;",
+            srcID, dstID);
     }
 
     void commitOrRollbackConnectionAndInitDBIfNecessary(
@@ -231,6 +242,36 @@ class DeleteRelTest : public DBTest {
         sortAndCheckTestResults(actualResult, expectedResult);
     }
 
+    void deleteRelsFromManyToOneTable(bool isCommit, TransactionTestType transactionTestType) {
+        conn->beginWriteTransaction();
+        conn->query(getDeleteTeachesRelQuery(11, 1));
+        conn->query(getDeleteTeachesRelQuery(31, 3));
+        commitOrRollbackConnectionAndInitDBIfNecessary(isCommit, transactionTestType);
+        auto result = conn->query("MATCH (:person)-[t:teaches]->(:person) return t.length");
+        auto actualResult = TestHelper::convertResultToString(*result);
+        vector<string> expectedResult = isCommit ?
+                                            vector<string>{"21", "22", "32", "33"} :
+                                            vector<string>{"11", "21", "22", "31", "32", "33"};
+        sortAndCheckTestResults(actualResult, expectedResult);
+    }
+
+    void deleteRelsFromOneToOneTable(bool isCommit, TransactionTestType transactionTestType) {
+        conn->beginWriteTransaction();
+        conn->query(getDeleteHasOwnerRelQuery(1, 51));
+        conn->query(getDeleteHasOwnerRelQuery(3, 53));
+        conn->query(getDeleteHasOwnerRelQuery(5, 55));
+        conn->query(getDeleteHasOwnerRelQuery(7, 57));
+        conn->query(getDeleteHasOwnerRelQuery(9, 59));
+        conn->query(getDeleteHasOwnerRelQuery(11, 61));
+        commitOrRollbackConnectionAndInitDBIfNecessary(isCommit, transactionTestType);
+        auto result = conn->query("MATCH (:animal)-[h:hasOwner]->(:person) return h.length");
+        auto actualResult = TestHelper::convertResultToString(*result);
+        vector<string> expectedResult =
+            isCommit ? vector<string>{"2", "4", "6", "8"} :
+                       vector<string>{"1", "2", "3", "4", "5", "6", "7", "8", "9", "11"};
+        sortAndCheckTestResults(actualResult, expectedResult);
+    }
+
     static constexpr uint64_t NUM_RELS_FOR_PERSON_KNOWS_PERSON = 2500;
     static constexpr uint64_t NUM_RELS_FOR_ANIMAL_KNOWS_PERSON = 51;
 };
@@ -421,3 +462,35 @@ TEST_F(DeleteRelTest, MixedDeleteAndCreateRels) {
         TestHelper::convertResultToString(*conn->query(knowsRelQuery), true /* checkOutputOrder */),
         expectedResult);
 }
+
+TEST_F(DeleteRelTest, DeleteRelsFromManyToOneTableCommitNormalExecution) {
+    deleteRelsFromManyToOneTable(true /* isCommit */, TransactionTestType::NORMAL_EXECUTION);
+}
+
+TEST_F(DeleteRelTest, DeleteRelsFromManyToOneTableCommitRecovery) {
+    deleteRelsFromManyToOneTable(true /* isCommit */, TransactionTestType::RECOVERY);
+}
+
+TEST_F(DeleteRelTest, DeleteRelsFromManyToOneTableRollbackNormalExecution) {
+    deleteRelsFromManyToOneTable(false /* isCommit */, TransactionTestType::NORMAL_EXECUTION);
+}
+
+TEST_F(DeleteRelTest, DeleteRelsFromManyToOneTableRollbackRecovery) {
+    deleteRelsFromManyToOneTable(false /* isCommit */, TransactionTestType::RECOVERY);
+}
+
+TEST_F(DeleteRelTest, DeleteRelsFromOneToOneTableCommitNormalExecution) {
+    deleteRelsFromOneToOneTable(true /* isCommit */, TransactionTestType::NORMAL_EXECUTION);
+}
+
+TEST_F(DeleteRelTest, DeleteRelsFromOneToOneTableCommitRecovery) {
+    deleteRelsFromOneToOneTable(true /* isCommit */, TransactionTestType::RECOVERY);
+}
+
+TEST_F(DeleteRelTest, DeleteRelsFromOneToOneTableRollbackNormalExecution) {
+    deleteRelsFromOneToOneTable(false /* isCommit */, TransactionTestType::NORMAL_EXECUTION);
+}
+
+TEST_F(DeleteRelTest, DeleteRelsFromOneToOneTableRollbackRecovery) {
+    deleteRelsFromOneToOneTable(false /* isCommit */, TransactionTestType::RECOVERY);
+}