Merge pull request #2465 from kuzudb/violation-check

Fix rel insert/copy violation check

src/include/storage/local_storage/local_rel_table.h

@@ -11,7 +11,7 @@ static constexpr common::column_id_t REL_ID_COLUMN_ID = 0;
 struct RelNGInfo {
     virtual ~RelNGInfo() = default;
 
-    virtual void insert(common::offset_t srcNodeOffset, common::offset_t relOffset,
+    virtual bool insert(common::offset_t srcNodeOffset, common::offset_t relOffset,
         common::row_idx_t adjNodeRowIdx,
         const std::vector<common::row_idx_t>& propertyNodesRowIdx) = 0;
     virtual void update(common::offset_t srcNodeOffset, common::offset_t relOffset,
@@ -39,7 +39,7 @@ struct RegularRelNGInfo final : public RelNGInfo {
         updateInfoPerChunk.resize(numChunks);
     }
 
-    void insert(common::offset_t srcNodeOffset, common::offset_t relOffset,
+    bool insert(common::offset_t srcNodeOffset, common::offset_t relOffset,
         common::row_idx_t adjNodeRowIdx,
         const std::vector<common::row_idx_t>& propertyNodesRowIdx) override;
     void update(common::offset_t srcNodeOffset, common::offset_t relOffset,
@@ -60,7 +60,7 @@ struct CSRRelNGInfo final : public RelNGInfo {
         updateInfoPerChunk.resize(numChunks);
     }
 
-    void insert(common::offset_t srcNodeOffset, common::offset_t relOffset,
+    bool insert(common::offset_t srcNodeOffset, common::offset_t relOffset,
         common::row_idx_t adjNodeRowIdx,
         const std::vector<common::row_idx_t>& propertyNodesRowIdx) override;
     void update(common::offset_t srcNodeOffset, common::offset_t relOffset,
@@ -73,7 +73,7 @@ class LocalRelNG final : public LocalNodeGroup {
     LocalRelNG(common::offset_t nodeGroupStartOffset, common::ColumnDataFormat dataFormat,
         std::vector<common::LogicalType*> dataTypes, MemoryManager* mm);
 
-    void insert(common::ValueVector* srcNodeIDVector, common::ValueVector* dstNodeIDVector,
+    bool insert(common::ValueVector* srcNodeIDVector, common::ValueVector* dstNodeIDVector,
         const std::vector<common::ValueVector*>& propertyVectors);
     void update(common::ValueVector* srcNodeIDVector, common::ValueVector* relIDVector,
         common::column_id_t columnID, common::ValueVector* propertyVector);
@@ -99,7 +99,7 @@ class LocalRelTableData final : public LocalTableData {
         common::ColumnDataFormat dataFormat)
         : LocalTableData{dataTypes, mm, dataFormat} {}
 
-    void insert(common::ValueVector* srcNodeIDVector, common::ValueVector* dstNodeIDVector,
+    bool insert(common::ValueVector* srcNodeIDVector, common::ValueVector* dstNodeIDVector,
         const std::vector<common::ValueVector*>& propertyVectors);
     void update(common::ValueVector* srcNodeIDVector, common::ValueVector* relIDVector,
         common::column_id_t columnID, common::ValueVector* propertyVector);

src/include/storage/store/column_chunk.h

@@ -71,7 +71,8 @@ class ColumnChunk {
     // `offsetInVector`, we should flatten the vector to pos at `offsetInVector`.
     virtual void write(common::ValueVector* vector, common::offset_t offsetInVector,
         common::offset_t offsetInChunk);
-    virtual void write(common::ValueVector* vector, common::ValueVector* offsetsInChunk);
+    virtual void write(
+        common::ValueVector* vector, common::ValueVector* offsetsInChunk, bool isCSR);
 
     // numValues must be at least the number of values the ColumnChunk was first initialized
     // with
@@ -142,7 +143,8 @@ class BoolColumnChunk : public ColumnChunk {
         uint32_t numValuesToAppend) override;
     void write(common::ValueVector* vector, common::offset_t offsetInVector,
         common::offset_t offsetInChunk) final;
-    void write(common::ValueVector* valueVector, common::ValueVector* offsetInChunkVector) final;
+    void write(common::ValueVector* valueVector, common::ValueVector* offsetInChunkVector,
+        bool isCSR) final;
 };
 
 class NullColumnChunk : public BoolColumnChunk {

src/include/storage/store/node_group.h

@@ -36,10 +36,19 @@ class NodeGroup {
 
     void finalize(uint64_t nodeGroupIdx_);
 
+    virtual inline void writeToColumnChunk(common::vector_idx_t chunkIdx,
+        common::vector_idx_t vectorIdx, common::DataChunk* dataChunk,
+        common::ValueVector* offsetVector) {
+        chunks[chunkIdx]->write(
+            dataChunk->getValueVector(vectorIdx).get(), offsetVector, false /* isCSR */);
+    }
+
+protected:
+    std::vector<std::unique_ptr<ColumnChunk>> chunks;
+
 private:
     uint64_t nodeGroupIdx;
     common::row_idx_t numRows;
-    std::vector<std::unique_ptr<ColumnChunk>> chunks;
 };
 
 class CSRNodeGroup : public NodeGroup {
@@ -55,6 +64,13 @@ class CSRNodeGroup : public NodeGroup {
 
     inline ColumnChunk* getCSROffsetChunk() { return csrOffsetChunk.get(); }
 
+    virtual inline void writeToColumnChunk(common::vector_idx_t chunkIdx,
+        common::vector_idx_t vectorIdx, common::DataChunk* dataChunk,
+        common::ValueVector* offsetVector) override {
+        chunks[chunkIdx]->write(
+            dataChunk->getValueVector(vectorIdx).get(), offsetVector, true /* isCSR */);
+    }
+
 private:
     std::unique_ptr<ColumnChunk> csrOffsetChunk;
 };

src/include/storage/store/string_column_chunk.h

@@ -30,7 +30,8 @@ class StringColumnChunk : public ColumnChunk {
 
     void write(common::ValueVector* vector, common::offset_t offsetInVector,
         common::offset_t offsetInChunk) final;
-    void write(common::ValueVector* valueVector, common::ValueVector* offsetInChunkVector) final;
+    void write(common::ValueVector* valueVector, common::ValueVector* offsetInChunkVector,
+        bool isCSR) final;
 
     template<typename T>
     T getValue(common::offset_t /*pos*/) const {

src/include/storage/store/struct_column_chunk.h

@@ -24,7 +24,8 @@ class StructColumnChunk : public ColumnChunk {
 
     void write(common::ValueVector* vector, common::offset_t offsetInVector,
         common::offset_t offsetInChunk) final;
-    void write(common::ValueVector* valueVector, common::ValueVector* offsetInChunkVector) final;
+    void write(common::ValueVector* valueVector, common::ValueVector* offsetInChunkVector,
+        bool isCSR) final;
 
     void resize(uint64_t newCapacity) final;
 

src/include/storage/store/var_list_column_chunk.h

@@ -40,7 +40,8 @@ class VarListColumnChunk : public ColumnChunk {
     // Note: `write` assumes that no `append` will be called afterward.
     void write(common::ValueVector* vector, common::offset_t offsetInVector,
         common::offset_t offsetInChunk) final;
-    void write(common::ValueVector* valueVector, common::ValueVector* offsetInChunkVector) final;
+    void write(common::ValueVector* valueVector, common::ValueVector* offsetInChunkVector,
+        bool isCSR) final;
 
     inline void resizeDataColumnChunk(uint64_t numBytesForBuffer) {
         // TODO(bmwinger): This won't work properly for booleans (will be one eighth as many values

src/storage/local_storage/local_rel_table.cpp

@@ -8,18 +8,19 @@ using namespace kuzu::common;
 namespace kuzu {
 namespace storage {
 
-void RegularRelNGInfo::insert(offset_t srcNodeOffset, offset_t /*relOffset*/,
+bool RegularRelNGInfo::insert(offset_t srcNodeOffset, offset_t /*relOffset*/,
     row_idx_t adjNodeRowIdx, const std::vector<row_idx_t>& propertyNodesRowIdx) {
     KU_ASSERT(propertyNodesRowIdx.size() == insertInfoPerChunk.size());
-    if (deleteInfo.contains(srcNodeOffset)) {
-        // We choose to ignore the insert operation if the node is deleted.
-        return;
-    }
+    bool isDeleted = deleteInfo.contains(srcNodeOffset);
     adjInsertInfo[srcNodeOffset] = adjNodeRowIdx;
+    if (updateInfoPerChunk[0].contains(srcNodeOffset) && !isDeleted) {
+        throw RuntimeException{"Many-one, one-one relationship violated."};
+    }
     for (auto i = 0u; i < propertyNodesRowIdx.size(); ++i) {
         KU_ASSERT(!updateInfoPerChunk[i].contains(srcNodeOffset));
         insertInfoPerChunk[i][srcNodeOffset] = propertyNodesRowIdx[i];
     }
+    return !isDeleted;
 }
 
 void RegularRelNGInfo::update(
@@ -56,12 +57,12 @@ bool RegularRelNGInfo::delete_(offset_t srcNodeOffset, offset_t /*relOffset*/) {
     return true;
 }
 
-void CSRRelNGInfo::insert(offset_t srcNodeOffset, offset_t relOffset, row_idx_t adjNodeRowIdx,
+bool CSRRelNGInfo::insert(offset_t srcNodeOffset, offset_t relOffset, row_idx_t adjNodeRowIdx,
     const std::vector<row_idx_t>& propertyNodesRowIdx) {
     KU_ASSERT(propertyNodesRowIdx.size() == insertInfoPerChunk.size());
     if (deleteInfo.contains(srcNodeOffset) && contains(deleteInfo.at(srcNodeOffset), relOffset)) {
         // We choose to ignore the insert operation if the node is deleted.
-        return;
+        return false;
     }
     if (adjInsertInfo.contains(srcNodeOffset)) {
         adjInsertInfo.at(srcNodeOffset)[relOffset] = adjNodeRowIdx;
@@ -75,6 +76,7 @@ void CSRRelNGInfo::insert(offset_t srcNodeOffset, offset_t relOffset, row_idx_t
             insertInfoPerChunk[i][srcNodeOffset] = {{relOffset, propertyNodesRowIdx[i]}};
         }
     }
+    return false;
 }
 
 void CSRRelNGInfo::update(
@@ -144,7 +146,7 @@ LocalRelNG::LocalRelNG(offset_t nodeGroupStartOffset, ColumnDataFormat dataForma
     adjChunk = std::make_unique<LocalVectorCollection>(LogicalType::INTERNAL_ID(), mm);
 }
 
-void LocalRelNG::insert(ValueVector* srcNodeIDVector, ValueVector* dstNodeIDVector,
+bool LocalRelNG::insert(ValueVector* srcNodeIDVector, ValueVector* dstNodeIDVector,
     const std::vector<ValueVector*>& propertyVectors) {
     KU_ASSERT(propertyVectors.size() == chunks.size() && propertyVectors.size() >= 1);
     auto adjNodeIDRowIdx = adjChunk->append(dstNodeIDVector);
@@ -159,7 +161,7 @@ void LocalRelNG::insert(ValueVector* srcNodeIDVector, ValueVector* dstNodeIDVect
     KU_ASSERT(srcNodeOffset < StorageConstants::NODE_GROUP_SIZE);
     auto relIDPos = propertyVectors[REL_ID_COLUMN_ID]->state->selVector->selectedPositions[0];
     auto relOffset = propertyVectors[REL_ID_COLUMN_ID]->getValue<relID_t>(relIDPos).offset;
-    relNGInfo->insert(srcNodeOffset, relOffset, adjNodeIDRowIdx, propertyValuesRowIdx);
+    return relNGInfo->insert(srcNodeOffset, relOffset, adjNodeIDRowIdx, propertyValuesRowIdx);
 }
 
 void LocalRelNG::update(ValueVector* srcNodeIDVector, ValueVector* relIDVector,
@@ -185,19 +187,19 @@ bool LocalRelNG::delete_(ValueVector* srcNodeIDVector, ValueVector* relIDVector)
     return relNGInfo->delete_(srcNodeOffset, relOffset);
 }
 
-void LocalRelTableData::insert(ValueVector* srcNodeIDVector, ValueVector* dstNodeIDVector,
+bool LocalRelTableData::insert(ValueVector* srcNodeIDVector, ValueVector* dstNodeIDVector,
     const std::vector<ValueVector*>& propertyVectors) {
     KU_ASSERT(srcNodeIDVector->state->selVector->selectedSize == 1 &&
               dstNodeIDVector->state->selVector->selectedSize == 1);
     auto srcNodeIDPos = srcNodeIDVector->state->selVector->selectedPositions[0];
     auto dstNodeIDPos = dstNodeIDVector->state->selVector->selectedPositions[0];
     if (srcNodeIDVector->isNull(srcNodeIDPos) || dstNodeIDVector->isNull(dstNodeIDPos)) {
-        return;
+        return false;
     }
     auto localNodeGroup =
         ku_dynamic_cast<LocalNodeGroup*, LocalRelNG*>(getOrCreateLocalNodeGroup(srcNodeIDVector));
     KU_ASSERT(localNodeGroup);
-    localNodeGroup->insert(srcNodeIDVector, dstNodeIDVector, propertyVectors);
+    return localNodeGroup->insert(srcNodeIDVector, dstNodeIDVector, propertyVectors);
 }
 
 void LocalRelTableData::update(ValueVector* srcNodeIDVector, ValueVector* relIDVector,

src/storage/store/column_chunk.cpp

@@ -1,5 +1,6 @@
 #include "storage/store/column_chunk.h"
 
+#include "common/exception/copy.h"
 #include "storage/compression/compression.h"
 #include "storage/storage_utils.h"
 #include "storage/store/string_column_chunk.h"
@@ -220,7 +221,7 @@ void ColumnChunk::append(
     numValues += numValuesToAppend;
 }
 
-void ColumnChunk::write(ValueVector* vector, ValueVector* offsetsInChunk) {
+void ColumnChunk::write(ValueVector* vector, ValueVector* offsetsInChunk, bool isCSR) {
     KU_ASSERT(
         vector->dataType.getPhysicalType() == dataType->getPhysicalType() &&
         offsetsInChunk->dataType.getPhysicalType() == PhysicalTypeID::INT64 &&
@@ -229,6 +230,11 @@ void ColumnChunk::write(ValueVector* vector, ValueVector* offsetsInChunk) {
     for (auto i = 0u; i < offsetsInChunk->state->selVector->selectedSize; i++) {
         auto offsetInChunk = offsets[offsetsInChunk->state->selVector->selectedPositions[i]];
         KU_ASSERT(offsetInChunk < capacity);
+        if (!isCSR && !nullChunk->isNull(offsetInChunk)) {
+            throw CopyException(stringFormat("Node with offset: {} can only have one neighbour due "
+                                             "to the MANY-ONE/ONE-ONE relationship constraint.",
+                offsetInChunk));
+        }
         auto offsetInVector = vector->state->selVector->selectedPositions[i];
         if (!vector->isNull(offsetInVector)) {
             memcpy(buffer.get() + offsetInChunk * numBytesPerValue,
@@ -373,7 +379,8 @@ void BoolColumnChunk::append(
     numValues += numValuesToAppend;
 }
 
-void BoolColumnChunk::write(ValueVector* valueVector, ValueVector* offsetInChunkVector) {
+void BoolColumnChunk::write(
+    ValueVector* valueVector, ValueVector* offsetInChunkVector, bool /*isCSR*/) {
     KU_ASSERT(valueVector->dataType.getPhysicalType() == PhysicalTypeID::BOOL &&
               offsetInChunkVector->dataType.getPhysicalType() == PhysicalTypeID::INT64 &&
               valueVector->state->selVector->selectedSize ==
@@ -431,7 +438,7 @@ class FixedListColumnChunk : public ColumnChunk {
         numValues += numValuesToAppend;
     }
 
-    void write(ValueVector* valueVector, ValueVector* offsetInChunkVector) final {
+    void write(ValueVector* valueVector, ValueVector* offsetInChunkVector, bool /*isCSR*/) final {
         KU_ASSERT(valueVector->dataType.getPhysicalType() == PhysicalTypeID::FIXED_LIST &&
                   offsetInChunkVector->dataType.getPhysicalType() == PhysicalTypeID::INT64);
         auto offsets = (offset_t*)offsetInChunkVector->getData();

src/storage/store/node_group.cpp

@@ -96,7 +96,9 @@ void NodeGroup::write(DataChunk* dataChunk, vector_idx_t offsetVectorIdx) {
             continue;
         }
         KU_ASSERT(vectorIdx < dataChunk->getNumValueVectors());
-        chunks[chunkIdx++]->write(dataChunk->getValueVector(vectorIdx++).get(), offsetVector);
+        writeToColumnChunk(chunkIdx, vectorIdx, dataChunk, offsetVector);
+        chunkIdx++;
+        vectorIdx++;
     }
     numRows += offsetVector->state->selVector->selectedSize;
 }

src/storage/store/rel_table_data.cpp

@@ -165,7 +165,14 @@ void RelTableData::insert(transaction::Transaction* transaction, ValueVector* sr
     auto localTableData = ku_dynamic_cast<LocalTableData*, LocalRelTableData*>(
         transaction->getLocalStorage()->getOrCreateLocalTableData(
             tableID, columns, TableType::REL, dataFormat, getDataIdxFromDirection(direction)));
-    localTableData->insert(srcNodeIDVector, dstNodeIDVector, propertyVectors);
+    auto checkPersistentStorage =
+        localTableData->insert(srcNodeIDVector, dstNodeIDVector, propertyVectors);
+    auto [nodeGroupIdx, offset] = StorageUtils::getNodeGroupIdxAndOffsetInChunk(
+        srcNodeIDVector->getValue<nodeID_t>(srcNodeIDVector->state->selVector->selectedPositions[0])
+            .offset);
+    if (checkPersistentStorage && !adjColumn->isNull(transaction, nodeGroupIdx, offset)) {
+        throw RuntimeException{"Many-one, one-one relationship violated."};
+    }
 }
 
 void RelTableData::update(transaction::Transaction* transaction, column_id_t columnID,

src/storage/store/string_column_chunk.cpp

@@ -76,7 +76,8 @@ void StringColumnChunk::write(
     }
 }
 
-void StringColumnChunk::write(ValueVector* valueVector, ValueVector* offsetInChunkVector) {
+void StringColumnChunk::write(
+    ValueVector* valueVector, ValueVector* offsetInChunkVector, bool /*isCSR*/) {
     KU_ASSERT(valueVector->dataType.getPhysicalType() == PhysicalTypeID::STRING &&
               offsetInChunkVector->dataType.getPhysicalType() == PhysicalTypeID::INT64 &&
               valueVector->state->selVector->selectedSize ==

src/storage/store/struct_column_chunk.cpp

@@ -64,7 +64,8 @@ void StructColumnChunk::write(
     }
 }
 
-void StructColumnChunk::write(ValueVector* valueVector, ValueVector* offsetInChunkVector) {
+void StructColumnChunk::write(
+    ValueVector* valueVector, ValueVector* offsetInChunkVector, bool isCSR) {
     KU_ASSERT(valueVector->dataType.getPhysicalType() == PhysicalTypeID::STRUCT);
     auto offsets = reinterpret_cast<offset_t*>(offsetInChunkVector->getData());
     for (auto i = 0u; i < offsetInChunkVector->state->selVector->selectedSize; i++) {
@@ -74,7 +75,7 @@ void StructColumnChunk::write(ValueVector* valueVector, ValueVector* offsetInChu
     }
     auto fields = StructVector::getFieldVectors(valueVector);
     for (auto i = 0u; i < fields.size(); i++) {
-        childChunks[i]->write(fields[i].get(), offsetInChunkVector);
+        childChunks[i]->write(fields[i].get(), offsetInChunkVector, isCSR);
     }
 }
 

src/storage/store/var_list_column_chunk.cpp

@@ -89,7 +89,8 @@ void VarListColumnChunk::appendNullList() {
     numValues++;
 }
 
-void VarListColumnChunk::write(ValueVector* valueVector, ValueVector* offsetInChunkVector) {
+void VarListColumnChunk::write(
+    ValueVector* valueVector, ValueVector* offsetInChunkVector, bool /*isCSR*/) {
     needFinalize = true;
     if (!indicesColumnChunk) {
         initializeIndices();

test/test_files/exceptions/copy/rel_multiplicity.test

@@ -1,20 +1,19 @@
 -GROUP CopyRelTableMultiplicityViolationTest
 -DATASET CSV copy-fault-tests/rel-table-multiplicity-violation
--SKIP
 
 --
 
 -CASE ManyOneMultiplicityViolationError
 -STATEMENT COPY knows FROM "${KUZU_ROOT_DIRECTORY}/dataset/copy-fault-tests/rel-table-multiplicity-violation/eKnows.csv"
 ---- error
-Copy exception: RelTable knows is a MANY_ONE table, but node(nodeOffset: 0) has more than one neighbour in the forward direction.
+Copy exception: Node with offset: 0 can only have one neighbour due to the MANY-ONE/ONE-ONE relationship constraint.
 
 -CASE OneManyMultiplicityViolationError
 -STATEMENT COPY teaches FROM "${KUZU_ROOT_DIRECTORY}/dataset/copy-fault-tests/rel-table-multiplicity-violation/eTeaches.csv"
 ---- error
-Copy exception: RelTable teaches is a ONE_MANY table, but node(nodeOffset: 2) has more than one neighbour in the backward direction.
+Copy exception: Node with offset: 2 can only have one neighbour due to the MANY-ONE/ONE-ONE relationship constraint.
 
 -CASE OneOneMultiplicityViolationError
 -STATEMENT COPY matches FROM "${KUZU_ROOT_DIRECTORY}/dataset/copy-fault-tests/rel-table-multiplicity-violation/eMatches.csv"
 ---- error
-Copy exception: RelTable matches is a ONE_ONE table, but node(nodeOffset: 1) has more than one neighbour in the forward direction.
+Copy exception: Node with offset: 1 can only have one neighbour due to the MANY-ONE/ONE-ONE relationship constraint.

test/test_files/transaction/create_rel/violate_error.test

@@ -1,6 +1,6 @@
 -GROUP CreateRelTest_ViolateError
 -DATASET CSV rel-update-tests
--SKIP
+
 --
 
 
@@ -9,11 +9,31 @@
 ---- ok
 -STATEMENT MATCH (p1:person), (p2:person) WHERE p1.ID = 11 AND p2.ID = 10 CREATE (p1)-[:teaches]->(p2);
 ---- error
-Runtime exception: Node in RelTable 4 cannot have more than one neighbour in the forward direction.
+Runtime exception: Many-one, one-one relationship violated.
 
 -CASE ViolateOneOneMultiplicityError
 -STATEMENT BEGIN TRANSACTION
 ---- ok
 -STATEMENT MATCH (a:animal), (p:person) WHERE a.ID = 2 AND p.ID = 10 CREATE (a)-[:hasOwner]->(p);
 ---- error
-Runtime exception: Node in RelTable 3 cannot have more than one neighbour in the forward direction.
+Runtime exception: Many-one, one-one relationship violated.
+
+-CASE InsertAfterDeleteRel
+-STATEMENT BEGIN TRANSACTION
+---- ok
+-STATEMENT MATCH (p1:person)-[r:teaches]->(p2:person) WHERE p1.ID = 11 AND p2.ID = 1 DELETE r;
+---- ok
+-STATEMENT MATCH (p1:person), (p2:person) WHERE p1.ID = 11 AND p2.ID = 10 CREATE (p1)-[:teaches]->(p2);
+---- ok
+-STATEMENT MATCH (p1:person), (p2:person) WHERE p1.ID = 11 AND p2.ID = 10 RETURN count(*);
+---- 1
+1
+-STATEMENT MATCH (p1:person)-[r:teaches]->(p2:person) WHERE p1.ID = 11 AND p2.ID = 10 DELETE r;
+---- ok
+-STATEMENT MATCH (p1:person), (p2:person) WHERE p1.ID = 11 AND p2.ID = 12 CREATE (p1)-[:teaches]->(p2);
+---- ok
+-STATEMENT MATCH (p1:person), (p2:person) WHERE p1.ID = 11 AND p2.ID = 12 RETURN count(*);
+---- 1
+1
+-STATEMENT COMMIT
+