From 4098bfa69f3c9735d6b1d8cb421855d613fb5ecb Mon Sep 17 00:00:00 2001 From: ziyi chen Date: Thu, 13 Jul 2023 13:46:51 -0400 Subject: [PATCH] Fix create node bug --- src/binder/bind/bind_updating_clause.cpp | 13 +++++++++++- src/catalog/catalog.cpp | 4 ---- src/include/catalog/catalog.h | 6 ++++-- src/include/catalog/catalog_structs.h | 4 +--- .../storage/storage_structure/column.h | 7 ++++++- src/main/connection.cpp | 2 +- src/processor/result/factorized_table.cpp | 1 + src/storage/storage_structure/column.cpp | 20 +++++++++++++++++++ src/storage/store/node_table.cpp | 2 +- test/test_files/tck/match/match1.test | 13 +++++------- 10 files changed, 51 insertions(+), 21 deletions(-) diff --git a/src/binder/bind/bind_updating_clause.cpp b/src/binder/bind/bind_updating_clause.cpp index 3619b21d11..3d8ba80c42 100644 --- a/src/binder/bind/bind_updating_clause.cpp +++ b/src/binder/bind/bind_updating_clause.cpp @@ -65,12 +65,23 @@ std::unique_ptr Binder::bindCreateNode( auto primaryKey = nodeTableSchema->getPrimaryKey(); std::shared_ptr primaryKeyExpression; std::vector setItems; + for (auto& property : catalog.getReadOnlyVersion()->getNodeProperties(nodeTableID)) { + if (collection.hasKeyVal(node, property.name)) { + setItems.emplace_back(collection.getKeyVal(node, property.name)); + } else { + auto propertyExpression = + expressionBinder.bindNodePropertyExpression(*node, property.name); + auto nullExpression = expressionBinder.createNullLiteralExpression(); + nullExpression = ExpressionBinder::implicitCastIfNecessary( + nullExpression, propertyExpression->dataType); + setItems.emplace_back(std::move(propertyExpression), std::move(nullExpression)); + } + } for (auto& [key, val] : collection.getKeyVals(node)) { auto propertyExpression = static_pointer_cast(key); if (propertyExpression->getPropertyID(nodeTableID) == primaryKey.propertyID) { primaryKeyExpression = val; } - setItems.emplace_back(key, val); } if (nodeTableSchema->getPrimaryKey().dataType.getLogicalTypeID() != LogicalTypeID::SERIAL && primaryKeyExpression == nullptr) { diff --git a/src/catalog/catalog.cpp b/src/catalog/catalog.cpp index 7d093b87f2..3438f38106 100644 --- a/src/catalog/catalog.cpp +++ b/src/catalog/catalog.cpp @@ -249,10 +249,6 @@ const Property& CatalogContent::getRelProperty( throw CatalogException("Cannot find rel property " + propertyName + "."); } -std::vector CatalogContent::getAllNodeProperties(table_id_t tableID) const { - return nodeTableSchemas.at(tableID)->getAllNodeProperties(); -} - void CatalogContent::dropTableSchema(table_id_t tableID) { auto tableSchema = getTableSchema(tableID); if (tableSchema->isNodeTable) { diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h index 31b10d3bd4..1dfce865f8 100644 --- a/src/include/catalog/catalog.h +++ b/src/include/catalog/catalog.h @@ -102,9 +102,11 @@ class CatalogContent { const Property& getRelProperty( common::table_id_t tableID, const std::string& propertyName) const; - std::vector getAllNodeProperties(common::table_id_t tableID) const; + inline const std::vector& getNodeProperties(common::table_id_t tableID) const { + return nodeTableSchemas.at(tableID)->getProperties(); + } inline const std::vector& getRelProperties(common::table_id_t tableID) const { - return relTableSchemas.at(tableID)->properties; + return relTableSchemas.at(tableID)->getProperties(); } inline std::vector getNodeTableIDs() const { std::vector nodeTableIDs; diff --git a/src/include/catalog/catalog_structs.h b/src/include/catalog/catalog_structs.h index 802c1f1aff..1e4ecf6335 100644 --- a/src/include/catalog/catalog_structs.h +++ b/src/include/catalog/catalog_structs.h @@ -63,7 +63,7 @@ struct TableSchema { return std::any_of(properties.begin(), properties.end(), [&propertyName](const Property& property) { return property.name == propertyName; }); } - + inline const std::vector& getProperties() const { return properties; } inline void addProperty(std::string propertyName, common::LogicalType dataType) { properties.emplace_back( std::move(propertyName), std::move(dataType), increaseNextPropertyID(), tableID); @@ -102,8 +102,6 @@ struct NodeTableSchema : TableSchema { inline Property getPrimaryKey() const { return properties[primaryKeyPropertyID]; } - inline std::vector getAllNodeProperties() const { return properties; } - // TODO(Semih): When we support updating the schemas, we need to update this or, we need // a more robust mechanism to keep track of which property is the primary key (e.g., store this // information with the property). This is an idx, not an ID, so as the columns/properties of diff --git a/src/include/storage/storage_structure/column.h b/src/include/storage/storage_structure/column.h index e7c66e9cfa..cd320232cf 100644 --- a/src/include/storage/storage_structure/column.h +++ b/src/include/storage/storage_structure/column.h @@ -39,7 +39,7 @@ class Column : public BaseColumnOrList { void write(common::ValueVector* nodeIDVector, common::ValueVector* vectorToWriteFrom); bool isNull(common::offset_t nodeOffset, transaction::Transaction* transaction); - void setNull(common::offset_t nodeOffset); + virtual void setNull(common::offset_t nodeOffset); inline NullColumn* getNullColumn() { return nullColumn.get(); } @@ -175,6 +175,11 @@ class StructPropertyColumn : public Column { void read(transaction::Transaction* transaction, common::ValueVector* nodeIDVector, common::ValueVector* resultVector) final; + void setNull(common::offset_t nodeOffset) final; + + void write(common::offset_t nodeOffset, common::ValueVector* vectorToWriteFrom, + uint32_t posInVectorToWriteFrom) override; + private: std::vector> structFieldColumns; }; diff --git a/src/main/connection.cpp b/src/main/connection.cpp index 9cbfbf81d7..20ec644560 100644 --- a/src/main/connection.cpp +++ b/src/main/connection.cpp @@ -245,7 +245,7 @@ std::string Connection::getNodePropertyNames(const std::string& tableName) { auto tableID = catalog->getReadOnlyVersion()->getTableID(tableName); auto primaryKeyPropertyID = catalog->getReadOnlyVersion()->getNodeTableSchema(tableID)->getPrimaryKey().propertyID; - for (auto& property : catalog->getReadOnlyVersion()->getAllNodeProperties(tableID)) { + for (auto& property : catalog->getReadOnlyVersion()->getNodeProperties(tableID)) { result += "\t" + property.name + " " + LogicalTypeUtils::dataTypeToString(property.dataType); result += property.propertyID == primaryKeyPropertyID ? "(PRIMARY KEY)\n" : "\n"; diff --git a/src/processor/result/factorized_table.cpp b/src/processor/result/factorized_table.cpp index c8be556a9f..6995ae328c 100644 --- a/src/processor/result/factorized_table.cpp +++ b/src/processor/result/factorized_table.cpp @@ -351,6 +351,7 @@ void FactorizedTable::setOverflowColNull( // TODO(Guodong): change this function to not use dataChunkPos in ColumnSchema. uint64_t FactorizedTable::computeNumTuplesToAppend( const std::vector& vectorsToAppend) const { + assert(!vectorsToAppend.empty()); auto unflatDataChunkPos = -1ul; auto numTuplesToAppend = 1ul; for (auto i = 0u; i < vectorsToAppend.size(); i++) { diff --git a/src/storage/storage_structure/column.cpp b/src/storage/storage_structure/column.cpp index 18010b114f..80fdbfc952 100644 --- a/src/storage/storage_structure/column.cpp +++ b/src/storage/storage_structure/column.cpp @@ -86,6 +86,9 @@ bool Column::isNull(common::offset_t nodeOffset, transaction::Transaction* trans void Column::setNull(common::offset_t nodeOffset) { nullColumn->setValue(nodeOffset); + auto walPageInfo = createWALVersionOfPageIfNecessaryForElement(nodeOffset, numElementsPerPage); + bufferManager->unpin(*wal->fileHandle, walPageInfo.pageIdxInWAL); + fileHandle->releaseWALPageIdxLock(walPageInfo.originalPageIdx); } Value Column::readValueForTestingOnly(offset_t offset) { @@ -325,6 +328,23 @@ void StructPropertyColumn::read(Transaction* transaction, common::ValueVector* n } } +void StructPropertyColumn::setNull(common::offset_t nodeOffset) { + nullColumn->setValue(nodeOffset); + // TODO(Ziyi): we currently only support create a node/rel with null struct property because + // we don't have a mechanism to log the sub columns of a struct column. +} + +void StructPropertyColumn::write(common::offset_t nodeOffset, + common::ValueVector* vectorToWriteFrom, uint32_t posInVectorToWriteFrom) { + bool isNull = vectorToWriteFrom->isNull(posInVectorToWriteFrom); + if (nullColumn) { + nullColumn->write(nodeOffset, vectorToWriteFrom, posInVectorToWriteFrom); + } + if (!isNull) { + throw common::RuntimeException{"Can only update a null value to struct column."}; + } +} + void InternalIDColumn::readInternalIDsFromPage(transaction::Transaction* transaction, uint8_t* frame, PageElementCursor& pageCursor, common::ValueVector* resultVector, uint32_t posInVector, uint32_t numValuesToRead, DiskOverflowFile* diskOverflowFile) { diff --git a/src/storage/store/node_table.cpp b/src/storage/store/node_table.cpp index d7103f8d65..2f9e18390a 100644 --- a/src/storage/store/node_table.cpp +++ b/src/storage/store/node_table.cpp @@ -16,7 +16,7 @@ NodeTable::NodeTable(NodesStatisticsAndDeletedIDs* nodesStatisticsAndDeletedIDs, std::unordered_map> NodeTable::initializeColumns( WAL* wal, kuzu::storage::BufferManager* bm, NodeTableSchema* nodeTableSchema) { std::unordered_map> propertyColumns; - for (auto& property : nodeTableSchema->getAllNodeProperties()) { + for (auto& property : nodeTableSchema->getProperties()) { propertyColumns[property.propertyID] = ColumnFactory::getColumn( StorageUtils::getNodePropertyColumnStructureIDAndFName(wal->getDirectory(), property), property.dataType, bm, wal); diff --git a/test/test_files/tck/match/match1.test b/test/test_files/tck/match/match1.test index 97c2b9b2aa..3358f34e8c 100644 --- a/test/test_files/tck/match/match1.test +++ b/test/test_files/tck/match/match1.test @@ -10,9 +10,7 @@ Binder exception: No node table exists in database. # Matching all nodes -# TODO(Guodong/Ziyi): Fixme -CASE Scenario2 --SKIP -STATEMENT CREATE NODE TABLE A(ID SERIAL, name STRING, PRIMARY KEY(ID)); ---- ok -STATEMENT CREATE NODE TABLE B(ID SERIAL, name STRING, PRIMARY KEY(ID)); @@ -21,13 +19,11 @@ Binder exception: No node table exists in database. ---- ok -STATEMENT MATCH (n) RETURN n; ---- 2 -1 -2 +{_ID: 0:0, _LABEL: A, ID: 0} +{_ID: 1:0, _LABEL: B, ID: 0, name: b} # Matching nodes using multiple labels -# TODO(Guodong/Ziyi): Fixme -CASE Scenario3 --SKIP -STATEMENT CREATE NODE TABLE A(ID SERIAL, PRIMARY KEY(ID)); ---- ok -STATEMENT CREATE NODE TABLE B(ID SERIAL, PRIMARY KEY(ID)); @@ -37,8 +33,9 @@ Binder exception: No node table exists in database. -STATEMENT CREATE (:A), (:B), (:C); ---- ok -STATEMENT MATCH (a:A:B) RETURN a; ----- 1 -1 +---- 2 +{_ID: 0:0, _LABEL: A, ID: 0} +{_ID: 1:0, _LABEL: B, ID: 0} # Simple node inlnie property predicate -CASE Scenario4