Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix add node property #1923

Merged
merged 1 commit into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/expression_evaluator/literal_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void LiteralExpressionEvaluator::resolveResultVector(
} else {
copyValueToVector(resultVector->getData(), resultVector.get(), value.get());
}
resultVector->state = DataChunkState::getSingleValueDataChunkState();
resultVector->setState(DataChunkState::getSingleValueDataChunkState());
}

void LiteralExpressionEvaluator::copyValueToVector(
Expand Down
1 change: 1 addition & 0 deletions src/include/main/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class Connection {
friend class kuzu::testing::TestHelper;
friend class kuzu::testing::TestRunner;
friend class kuzu::benchmark::Benchmark;
friend class kuzu::testing::TinySnbDDLTest;

public:
/**
Expand Down
2 changes: 1 addition & 1 deletion src/include/processor/operator/ddl/add_node_property.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class AddNodeProperty : public AddProperty {
: AddProperty{catalog, tableID, std::move(propertyName), std::move(dataType),
std::move(defaultValueEvaluator), storageManager, outputPos, id, paramsString} {}

void executeDDLInternal() override;
void executeDDLInternal() final;

std::unique_ptr<PhysicalOperator> clone() override {
return make_unique<AddNodeProperty>(catalog, tableID, propertyName, dataType->copy(),
Expand Down
7 changes: 1 addition & 6 deletions src/include/processor/operator/ddl/add_property.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,7 @@ class AddProperty : public DDL {
std::string getOutputMsg() override { return {"Add Succeed."}; }

protected:
inline bool isDefaultValueNull() const {
auto expressionVector = defaultValueEvaluator->resultVector;
return expressionVector->isNull(expressionVector->state->selVector->selectedPositions[0]);
}

uint8_t* getDefaultVal();
common::ValueVector* getDefaultValVector();

protected:
common::table_id_t tableID;
Expand Down
15 changes: 13 additions & 2 deletions src/include/storage/copier/column_chunk.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class ColumnChunk {
// Include pages for null and children segments.
virtual common::page_idx_t getNumPages() const;

virtual void append(common::ValueVector* vector, common::offset_t startPosInChunk);

void append(
common::ValueVector* vector, common::offset_t startPosInChunk, uint32_t numValuesToAppend);

Expand Down Expand Up @@ -86,13 +88,16 @@ class ColumnChunk {
((T*)buffer.get())[pos] = val;
}

void populateWithDefaultVal(common::ValueVector* defaultValueVector);

protected:
// Initializes the data buffer. Is (and should be) only called in constructor.
virtual void initialize(common::offset_t numValues);

template<typename T>
void templateCopyArrowArray(
arrow::Array* array, common::offset_t startPosInChunk, uint32_t numValuesToAppend);

// TODO(Guodong/Ziyi): The conversion from string to values should be handled inside ReadFile.
template<typename T>
void templateCopyValuesAsString(
Expand All @@ -104,6 +109,8 @@ class ColumnChunk {

common::offset_t getOffsetInBuffer(common::offset_t pos) const;

void copyVectorToBuffer(common::ValueVector* vector, common::offset_t startPosInChunk);

protected:
common::LogicalType dataType;
uint32_t numBytesPerValue;
Expand Down Expand Up @@ -132,6 +139,11 @@ class BoolColumnChunk : public ColumnChunk {
: ColumnChunk(
common::LogicalType(common::LogicalTypeID::BOOL), copyDescription, hasNullChunk) {}

void append(common::ValueVector* vector, common::offset_t startPosInChunk) final;

void append(
arrow::Array* array, common::offset_t startPosInChunk, uint32_t numValuesToAppend) final;

void append(ColumnChunk* other, common::offset_t startPosInOtherChunk,
common::offset_t startPosInChunk, uint32_t numValuesToAppend) final;
};
Expand All @@ -145,8 +157,6 @@ class NullColumnChunk : public BoolColumnChunk {

void resize(uint64_t numValues) final;

void setRangeNoNull(common::offset_t startPosInChunk, uint32_t numValuesToSet);

inline void resetNullBuffer() { memset(buffer.get(), 0 /* non null */, numBytes); }

protected:
Expand Down Expand Up @@ -202,5 +212,6 @@ void ColumnChunk::setValueFromString<common::date_t>(
template<>
void ColumnChunk::setValueFromString<common::timestamp_t>(
const char* value, uint64_t length, uint64_t pos);

} // namespace storage
} // namespace kuzu
1 change: 1 addition & 0 deletions src/include/storage/copier/string_column_chunk.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class StringColumnChunk : public ColumnChunk {
StringColumnChunk(common::LogicalType dataType, common::CopyDescription* copyDescription);

void resetToEmpty() final;
void append(common::ValueVector* vector, common::offset_t startPosInChunk) final;
void append(
arrow::Array* array, common::offset_t startPosInChunk, uint32_t numValuesToAppend) final;
void append(ColumnChunk* other, common::offset_t startPosInOtherChunk,
Expand Down
1 change: 1 addition & 0 deletions src/include/storage/copier/struct_column_chunk.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class StructColumnChunk : public ColumnChunk {
arrow::Array* array, common::offset_t startPosInChunk, uint32_t numValuesToAppend) final;
void append(ColumnChunk* other, common::offset_t startPosInOtherChunk,
common::offset_t startPosInChunk, uint32_t numValuesToAppend) final;
void append(common::ValueVector* vector, common::offset_t startPosInChunk) final;

private:
// TODO(Guodong): These methods are duplicated from `InMemStructColumnChunk`, which will be
Expand Down
2 changes: 2 additions & 0 deletions src/include/storage/copier/var_list_column_chunk.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class VarListColumnChunk : public ColumnChunk {

void resetToEmpty() final;

void append(common::ValueVector* vector, common::offset_t startPosInChunk) final;

private:
inline common::page_idx_t getNumPages() const final {
return varListDataColumnChunk.dataChunk->getNumPages() + ColumnChunk::getNumPages();
Expand Down
9 changes: 6 additions & 3 deletions src/include/storage/storage_structure/disk_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ class BaseDiskArray {
BaseDiskArray(FileHandle& fileHandle, common::page_idx_t headerPageIdx, uint64_t elementSize);
// Used when loading from file
BaseDiskArray(FileHandle& fileHandle, StorageStructureID storageStructureID,
common::page_idx_t headerPageIdx, BufferManager* bufferManager, WAL* wal);
common::page_idx_t headerPageIdx, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction);

virtual ~BaseDiskArray() = default;

Expand Down Expand Up @@ -193,7 +194,8 @@ class BaseInMemDiskArray : public BaseDiskArray<U> {
BaseInMemDiskArray(
FileHandle& fileHandle, common::page_idx_t headerPageIdx, uint64_t elementSize);
BaseInMemDiskArray(FileHandle& fileHandle, StorageStructureID storageStructureID,
common::page_idx_t headerPageIdx, BufferManager* bufferManager, WAL* wal);
common::page_idx_t headerPageIdx, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction);

public:
// [] operator can be used to update elements, e.g., diskArray[5] = 4, when building an
Expand Down Expand Up @@ -229,7 +231,8 @@ template<typename T>
class InMemDiskArray : public BaseInMemDiskArray<T> {
public:
InMemDiskArray(FileHandle& fileHandle, StorageStructureID storageStructureID,
common::page_idx_t headerPageIdx, BufferManager* bufferManager, WAL* wal);
common::page_idx_t headerPageIdx, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction);

static inline common::page_idx_t addDAHPageToFile(BMFileHandle& fileHandle,
StorageStructureID storageStructureID, BufferManager* bufferManager, WAL* wal) {
Expand Down
24 changes: 16 additions & 8 deletions src/include/storage/store/node_column.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ class NullNodeColumn;
class NodeColumn {
public:
NodeColumn(const catalog::Property& property, BMFileHandle* dataFH, BMFileHandle* metadataFH,
BufferManager* bufferManager, WAL* wal, bool requireNullColumn = true);
BufferManager* bufferManager, WAL* wal, transaction::Transaction* transaction,
bool requireNullColumn = true);
NodeColumn(common::LogicalType dataType, const catalog::MetadataDAHInfo& metaDAHeaderInfo,
BMFileHandle* dataFH, BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
bool requireNullColumn);
transaction::Transaction* transaction, bool requireNullColumn);
virtual ~NodeColumn() = default;

// Expose for feature store
Expand Down Expand Up @@ -88,6 +89,9 @@ class NodeColumn {
virtual void checkpointInMemory();
virtual void rollbackInMemory();

void populateWithDefaultVal(const catalog::Property& property, NodeColumn* nodeColumn,
common::ValueVector* defaultValueVector, uint64_t numNodeGroups);

protected:
virtual void scanInternal(transaction::Transaction* transaction,
common::ValueVector* nodeIDVector, common::ValueVector* resultVector);
Expand Down Expand Up @@ -132,15 +136,16 @@ class BoolNodeColumn : public NodeColumn {
public:
BoolNodeColumn(const catalog::MetadataDAHInfo& metaDAHeaderInfo, BMFileHandle* dataFH,
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
bool requireNullColumn = true);
transaction::Transaction* transaction, bool requireNullColumn = true);

void batchLookup(const common::offset_t* nodeOffsets, size_t size, uint8_t* result) final;
};

class NullNodeColumn : public NodeColumn {
public:
NullNodeColumn(common::page_idx_t metaDAHPageIdx, BMFileHandle* dataFH,
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal);
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction);

void scan(transaction::Transaction* transaction, common::ValueVector* nodeIDVector,
common::ValueVector* resultVector) final;
Expand All @@ -158,7 +163,8 @@ class NullNodeColumn : public NodeColumn {
class SerialNodeColumn : public NodeColumn {
public:
SerialNodeColumn(const catalog::MetadataDAHInfo& metaDAHeaderInfo, BMFileHandle* dataFH,
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal);
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction);

void scan(transaction::Transaction* transaction, common::ValueVector* nodeIDVector,
common::ValueVector* resultVector) final;
Expand All @@ -170,13 +176,15 @@ class SerialNodeColumn : public NodeColumn {

struct NodeColumnFactory {
static inline std::unique_ptr<NodeColumn> createNodeColumn(const catalog::Property& property,
BMFileHandle* dataFH, BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal) {
BMFileHandle* dataFH, BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction) {
return createNodeColumn(*property.getDataType(), *property.getMetadataDAHInfo(), dataFH,
metadataFH, bufferManager, wal);
metadataFH, bufferManager, wal, transaction);
}
static std::unique_ptr<NodeColumn> createNodeColumn(const common::LogicalType& dataType,
const catalog::MetadataDAHInfo& metaDAHeaderInfo, BMFileHandle* dataFH,
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal);
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction);
};

} // namespace storage
Expand Down
11 changes: 3 additions & 8 deletions src/include/storage/store/node_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,9 @@ class NodeTable {
inline common::property_id_t getPKPropertyID() const { return pkPropertyID; }
inline common::table_id_t getTableID() const { return tableID; }

inline void dropProperty(common::property_id_t propertyID) {
propertyColumns.erase(propertyID);
}
inline void addProperty(const catalog::Property& property) {
assert(!propertyColumns.contains(property.getPropertyID()));
propertyColumns.emplace(property.getPropertyID(),
NodeColumnFactory::createNodeColumn(property, dataFH, dataFH, &bufferManager, wal));
}
inline void dropColumn(common::property_id_t propertyID) { propertyColumns.erase(propertyID); }
void addColumn(const catalog::Property& property, common::ValueVector* defaultValueVector,
transaction::Transaction* transaction);

void prepareCommit();
void prepareRollback();
Expand Down
3 changes: 2 additions & 1 deletion src/include/storage/store/string_node_column.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ struct StringNodeColumnFunc {
class StringNodeColumn : public NodeColumn {
public:
StringNodeColumn(common::LogicalType dataType, const catalog::MetadataDAHInfo& metaDAHeaderInfo,
BMFileHandle* dataFH, BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal);
BMFileHandle* dataFH, BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction);

void scan(transaction::Transaction* transaction, common::node_group_idx_t nodeGroupIdx,
common::offset_t startOffsetInGroup, common::offset_t endOffsetInGroup,
Expand Down
3 changes: 2 additions & 1 deletion src/include/storage/store/struct_node_column.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ namespace storage {
class StructNodeColumn : public NodeColumn {
public:
StructNodeColumn(common::LogicalType dataType, const catalog::MetadataDAHInfo& metaDAHeaderInfo,
BMFileHandle* dataFH, BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal);
BMFileHandle* dataFH, BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction);

void scan(transaction::Transaction* transaction, common::node_group_idx_t nodeGroupIdx,
common::offset_t startOffsetInGroup, common::offset_t endOffsetInGroup,
Expand Down
11 changes: 6 additions & 5 deletions src/include/storage/store/var_list_node_column.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ class VarListNodeColumn : public NodeColumn {
public:
VarListNodeColumn(common::LogicalType dataType,
const catalog::MetadataDAHInfo& metaDAHeaderInfo, BMFileHandle* dataFH,
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal)
BMFileHandle* metadataFH, BufferManager* bufferManager, WAL* wal,
transaction::Transaction* transaction)
: NodeColumn{std::move(dataType), metaDAHeaderInfo, dataFH, metadataFH, bufferManager, wal,
true /* requireNullColumn */} {
dataNodeColumn =
NodeColumnFactory::createNodeColumn(*common::VarListType::getChildType(&this->dataType),
*metaDAHeaderInfo.childrenInfos[0], dataFH, metadataFH, bufferManager, wal);
transaction, true /* requireNullColumn */} {
dataNodeColumn = NodeColumnFactory::createNodeColumn(
*common::VarListType::getChildType(&this->dataType), *metaDAHeaderInfo.childrenInfos[0],
dataFH, metadataFH, bufferManager, wal, transaction);
}

void scan(transaction::Transaction* transaction, common::node_group_idx_t nodeGroupIdx,
Expand Down
3 changes: 3 additions & 0 deletions src/processor/operator/ddl/add_node_property.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ void AddNodeProperty::executeDDLInternal() {
auto metadataDAHInfo = storageManager.createMetadataDAHInfo(*dataType);
catalog->addNodeProperty(
tableID, propertyName, std::move(dataType), std::move(metadataDAHInfo));
auto addedProp = catalog->getWriteVersion()->getNodeProperty(tableID, propertyName);
storageManager.getNodesStore().getNodeTable(tableID)->addColumn(
*addedProp, getDefaultValVector(), transaction);
}

} // namespace processor
Expand Down
8 changes: 2 additions & 6 deletions src/processor/operator/ddl/add_property.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,9 @@
namespace kuzu {
namespace processor {

uint8_t* AddProperty::getDefaultVal() {
common::ValueVector* AddProperty::getDefaultValVector() {
defaultValueEvaluator->evaluate();
auto expressionVector = defaultValueEvaluator->resultVector;
assert(defaultValueEvaluator->resultVector->dataType == *dataType);
auto posInExpressionVector = expressionVector->state->selVector->selectedPositions[0];
return expressionVector->getData() +
expressionVector->getNumBytesPerValue() * posInExpressionVector;
return defaultValueEvaluator->resultVector.get();
}

} // namespace processor
Expand Down
8 changes: 6 additions & 2 deletions src/processor/operator/ddl/add_rel_property.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@ void AddRelProperty::executeDDLInternal() {
catalog->addRelProperty(tableID, propertyName, dataType->copy());
auto tableSchema = catalog->getWriteVersion()->getRelTableSchema(tableID);
auto property = tableSchema->getProperty(tableSchema->getPropertyID(propertyName));
StorageUtils::createFileForRelPropertyWithDefaultVal(
tableSchema, *property, getDefaultVal(), isDefaultValueNull(), storageManager);
auto defaultValVector = getDefaultValVector();
auto posInDefaultValVector = defaultValVector->state->selVector->selectedPositions[0];
auto defaultVal = defaultValVector->getData() +
defaultValVector->getNumBytesPerValue() * posInDefaultValVector;
StorageUtils::createFileForRelPropertyWithDefaultVal(tableSchema, *property, defaultVal,
defaultValVector->isNull(posInDefaultValVector), storageManager);
}

} // namespace processor
Expand Down
Loading
Loading