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 struct pack bug #1515

Merged
merged 1 commit into from
May 5, 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
24 changes: 18 additions & 6 deletions src/expression_evaluator/function_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,7 @@ std::unique_ptr<BaseExpressionEvaluator> FunctionExpressionEvaluator::clone() {
void FunctionExpressionEvaluator::resolveResultVector(
const ResultSet& resultSet, MemoryManager* memoryManager) {
auto& functionExpression = (binder::ScalarFunctionExpression&)*expression;
if (functionExpression.getFunctionName() == STRUCT_PACK_FUNC_NAME) {
resultVector = std::make_shared<ValueVector>(expression->dataType, memoryManager);
for (auto& child : children) {
StructVector::addChildVector(resultVector.get(), child->resultVector);
}
} else if (functionExpression.getFunctionName() == STRUCT_EXTRACT_FUNC_NAME) {
if (functionExpression.getFunctionName() == STRUCT_EXTRACT_FUNC_NAME) {
auto& bindData = (function::StructExtractBindData&)*functionExpression.getBindData();
resultVector =
StructVector::getChildVector(children[0]->resultVector.get(), bindData.childIdx);
Expand All @@ -78,6 +73,23 @@ void FunctionExpressionEvaluator::resolveResultVector(
inputEvaluators.push_back(child.get());
}
resolveResultStateFromChildren(inputEvaluators);
if (functionExpression.getFunctionName() == STRUCT_PACK_FUNC_NAME) {
// Our goal is to make the state of the resultVector consistent with its children vectors.
// If the resultVector and inputVector are in different dataChunks, we should create a new
// child valueVector, which shares the state with the resultVector, instead of reusing the
// inputVector.
for (auto& inputEvaluator : inputEvaluators) {
if (inputEvaluator->resultVector->state != resultVector->state) {
auto structFieldVector = std::make_shared<common::ValueVector>(
inputEvaluator->resultVector->dataType, memoryManager);
structFieldVector->state = resultVector->state;
common::StructVector::addChildVector(resultVector.get(), structFieldVector);
} else {
common::StructVector::addChildVector(
resultVector.get(), inputEvaluator->resultVector);
}
}
}
}

} // namespace evaluator
Expand Down
36 changes: 35 additions & 1 deletion src/include/function/struct/vector_struct_operations.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include "common/vector/value_vector_utils.h"
#include "function/vector_operations.h"

namespace kuzu {
Expand All @@ -10,7 +11,40 @@ struct StructPackVectorOperations : public VectorOperations {
static std::unique_ptr<FunctionBindData> bindFunc(
const binder::expression_vector& arguments, FunctionDefinition* definition);
static void execFunc(const std::vector<std::shared_ptr<common::ValueVector>>& parameters,
common::ValueVector& result) {} // Evaluate at compile time
common::ValueVector& result) {
for (auto i = 0u; i < parameters.size(); i++) {
auto& parameter = parameters[i];
if (parameter->state == result.state) {
continue;
}
// If the parameter's state is inconsistent with the result's state, we need to copy the
// parameter's value to the corresponding child vector.
copyParameterValueToStructFieldVector(
parameter.get(), common::StructVector::getChildVector(&result, i).get());
}
}
static void copyParameterValueToStructFieldVector(
const common::ValueVector* parameter, common::ValueVector* structField) {
// If the parameter is unFlat, then its state must be consistent with the result's state.
// Thus, we don't need to copy values to structFieldVector.
assert(parameter->state->isFlat());
acquamarin marked this conversation as resolved.
Show resolved Hide resolved
auto srcValue =
parameter->getData() +
parameter->getNumBytesPerValue() * parameter->state->selVector->selectedPositions[0];
if (structField->state->isFlat()) {
common::ValueVectorUtils::copyValue(
structField->getData() + structField->getNumBytesPerValue() *
structField->state->selVector->selectedPositions[0],
*structField, srcValue, *parameter);
} else {
for (auto j = 0u; j < structField->state->selVector->selectedSize; j++) {
auto pos = structField->state->selVector->selectedPositions[j];
common::ValueVectorUtils::copyValue(
structField->getData() + structField->getNumBytesPerValue() * pos, *structField,
srcValue, *parameter);
}
}
}
};

struct StructExtractBindData : public FunctionBindData {
Expand Down
30 changes: 30 additions & 0 deletions test/test_files/tinysnb/projection/single_label.test
Original file line number Diff line number Diff line change
Expand Up @@ -439,3 +439,33 @@ Dan|Carol
{RATING: 1223.000000, VIEWS: 10003, RELEASE: 2011-02-11 16:44:22, FILM: 2013-02-22}
{RATING: 5.300000, VIEWS: 152, RELEASE: 2011-08-20 11:25:30, FILM: 2012-05-11}
{RATING: 7.000000, VIEWS: 982, RELEASE: 2018-11-13 13:33:11, FILM: 2014-09-12}

-NAME ReturnStructLiteralWithUnflatFlatChildren
-QUERY MATCH (p:person)-[e:knows]->(p1:person) return {name: p.fName, id: p1.ID, date: e.date}
---- 14
{NAME: Alice, ID: 2, DATE: 2021-06-30}
{NAME: Alice, ID: 3, DATE: 2021-06-30}
{NAME: Alice, ID: 5, DATE: 2021-06-30}
{NAME: Bob, ID: 0, DATE: 2021-06-30}
{NAME: Bob, ID: 3, DATE: 1950-05-14}
{NAME: Bob, ID: 5, DATE: 1950-05-14}
{NAME: Carol, ID: 0, DATE: 2021-06-30}
{NAME: Carol, ID: 2, DATE: 1950-05-14}
{NAME: Carol, ID: 5, DATE: 2000-01-01}
{NAME: Dan, ID: 0, DATE: 2021-06-30}
{NAME: Dan, ID: 2, DATE: 1950-05-14}
{NAME: Dan, ID: 3, DATE: 2000-01-01}
{NAME: Elizabeth, ID: 8, DATE: 1905-12-12}
{NAME: Elizabeth, ID: 9, DATE: 1905-12-12}

-NAME ReturnNestedStructLiteral
-QUERY MATCH (p:person) return {description: {gender: p.gender, age: p.age}, name: p.fName}
---- 8
{DESCRIPTION: {GENDER: 1, AGE: 20}, NAME: Elizabeth}
{DESCRIPTION: {GENDER: 1, AGE: 35}, NAME: Alice}
{DESCRIPTION: {GENDER: 1, AGE: 45}, NAME: Carol}
{DESCRIPTION: {GENDER: 2, AGE: 20}, NAME: Dan}
{DESCRIPTION: {GENDER: 2, AGE: 25}, NAME: Farooq}
{DESCRIPTION: {GENDER: 2, AGE: 30}, NAME: Bob}
{DESCRIPTION: {GENDER: 2, AGE: 40}, NAME: Greg}
{DESCRIPTION: {GENDER: 2, AGE: 83}, NAME: Hubert Blaine Wolfeschlegelsteinhausenbergerdorff}