From 3108a2184fdbece7cf488e16cc7fffc423def026 Mon Sep 17 00:00:00 2001 From: ziyi chen Date: Thu, 4 May 2023 21:22:36 -0400 Subject: [PATCH] Fix struct pack bug --- .../function_evaluator.cpp | 24 +++++++++---- .../struct/vector_struct_operations.h | 36 ++++++++++++++++++- .../tinysnb/projection/single_label.test | 30 ++++++++++++++++ 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/src/expression_evaluator/function_evaluator.cpp b/src/expression_evaluator/function_evaluator.cpp index 9de3530837..3f274bd14b 100644 --- a/src/expression_evaluator/function_evaluator.cpp +++ b/src/expression_evaluator/function_evaluator.cpp @@ -61,12 +61,7 @@ std::unique_ptr 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(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); @@ -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( + inputEvaluator->resultVector->dataType, memoryManager); + structFieldVector->state = resultVector->state; + common::StructVector::addChildVector(resultVector.get(), structFieldVector); + } else { + common::StructVector::addChildVector( + resultVector.get(), inputEvaluator->resultVector); + } + } + } } } // namespace evaluator diff --git a/src/include/function/struct/vector_struct_operations.h b/src/include/function/struct/vector_struct_operations.h index f90af3db1e..3636fe158a 100644 --- a/src/include/function/struct/vector_struct_operations.h +++ b/src/include/function/struct/vector_struct_operations.h @@ -1,5 +1,6 @@ #pragma once +#include "common/vector/value_vector_utils.h" #include "function/vector_operations.h" namespace kuzu { @@ -10,7 +11,40 @@ struct StructPackVectorOperations : public VectorOperations { static std::unique_ptr bindFunc( const binder::expression_vector& arguments, FunctionDefinition* definition); static void execFunc(const std::vector>& 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()); + 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 { diff --git a/test/test_files/tinysnb/projection/single_label.test b/test/test_files/tinysnb/projection/single_label.test index e968b0086f..31b543b44f 100644 --- a/test/test_files/tinysnb/projection/single_label.test +++ b/test/test_files/tinysnb/projection/single_label.test @@ -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}