Fix struct pack bug

kuzudb · May 5, 2023 · 3108a21 · 3108a21
1 parent 6c9b8ae
commit 3108a21
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 7 deletions.
diff --git a/src/expression_evaluator/function_evaluator.cpp b/src/expression_evaluator/function_evaluator.cpp
@@ -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);
@@ -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

diff --git 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<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());
+        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
@@ -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}