Skip to content

Commit

Permalink
Fix create node bug
Browse files Browse the repository at this point in the history
  • Loading branch information
acquamarin committed Jul 13, 2023
1 parent deb1734 commit 4098bfa
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 21 deletions.
13 changes: 12 additions & 1 deletion src/binder/bind/bind_updating_clause.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,23 @@ std::unique_ptr<BoundCreateNode> Binder::bindCreateNode(
auto primaryKey = nodeTableSchema->getPrimaryKey();
std::shared_ptr<Expression> primaryKeyExpression;
std::vector<expression_pair> 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<PropertyExpression>(key);
if (propertyExpression->getPropertyID(nodeTableID) == primaryKey.propertyID) {
primaryKeyExpression = val;
}
setItems.emplace_back(key, val);
}
if (nodeTableSchema->getPrimaryKey().dataType.getLogicalTypeID() != LogicalTypeID::SERIAL &&
primaryKeyExpression == nullptr) {
Expand Down
4 changes: 0 additions & 4 deletions src/catalog/catalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,6 @@ const Property& CatalogContent::getRelProperty(
throw CatalogException("Cannot find rel property " + propertyName + ".");
}

std::vector<Property> 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) {
Expand Down
6 changes: 4 additions & 2 deletions src/include/catalog/catalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,11 @@ class CatalogContent {
const Property& getRelProperty(
common::table_id_t tableID, const std::string& propertyName) const;

std::vector<Property> getAllNodeProperties(common::table_id_t tableID) const;
inline const std::vector<Property>& getNodeProperties(common::table_id_t tableID) const {
return nodeTableSchemas.at(tableID)->getProperties();
}
inline const std::vector<Property>& getRelProperties(common::table_id_t tableID) const {
return relTableSchemas.at(tableID)->properties;
return relTableSchemas.at(tableID)->getProperties();
}
inline std::vector<common::table_id_t> getNodeTableIDs() const {
std::vector<common::table_id_t> nodeTableIDs;
Expand Down
4 changes: 1 addition & 3 deletions src/include/catalog/catalog_structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Property>& getProperties() const { return properties; }
inline void addProperty(std::string propertyName, common::LogicalType dataType) {
properties.emplace_back(
std::move(propertyName), std::move(dataType), increaseNextPropertyID(), tableID);
Expand Down Expand Up @@ -102,8 +102,6 @@ struct NodeTableSchema : TableSchema {

inline Property getPrimaryKey() const { return properties[primaryKeyPropertyID]; }

inline std::vector<Property> 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
Expand Down
7 changes: 6 additions & 1 deletion src/include/storage/storage_structure/column.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }

Expand Down Expand Up @@ -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<std::unique_ptr<Column>> structFieldColumns;
};
Expand Down
2 changes: 1 addition & 1 deletion src/main/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
1 change: 1 addition & 0 deletions src/processor/result/factorized_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ValueVector*>& vectorsToAppend) const {
assert(!vectorsToAppend.empty());
auto unflatDataChunkPos = -1ul;
auto numTuplesToAppend = 1ul;
for (auto i = 0u; i < vectorsToAppend.size(); i++) {
Expand Down
20 changes: 20 additions & 0 deletions src/storage/storage_structure/column.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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."};

Check warning on line 344 in src/storage/storage_structure/column.cpp

View check run for this annotation

Codecov / codecov/patch

src/storage/storage_structure/column.cpp#L344

Added line #L344 was not covered by tests
}
}

void InternalIDColumn::readInternalIDsFromPage(transaction::Transaction* transaction,
uint8_t* frame, PageElementCursor& pageCursor, common::ValueVector* resultVector,
uint32_t posInVector, uint32_t numValuesToRead, DiskOverflowFile* diskOverflowFile) {
Expand Down
2 changes: 1 addition & 1 deletion src/storage/store/node_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ NodeTable::NodeTable(NodesStatisticsAndDeletedIDs* nodesStatisticsAndDeletedIDs,
std::unordered_map<common::property_id_t, std::unique_ptr<Column>> NodeTable::initializeColumns(
WAL* wal, kuzu::storage::BufferManager* bm, NodeTableSchema* nodeTableSchema) {
std::unordered_map<common::property_id_t, std::unique_ptr<Column>> 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);
Expand Down
13 changes: 5 additions & 8 deletions test/test_files/tck/match/match1.test
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand All @@ -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
Expand Down

0 comments on commit 4098bfa

Please sign in to comment.