From 8972927258db1014e1c9c246d6b88c1f658f43f0 Mon Sep 17 00:00:00 2001 From: xiyang Date: Thu, 30 Mar 2023 17:16:25 -0400 Subject: [PATCH] Fix issue-1428, 1408 --- src/binder/bind/bind_projection_clause.cpp | 1 - .../function_evaluator.cpp | 13 +++++++++++++ src/include/planner/projection_planner.h | 3 +++ src/planner/projection_planner.cpp | 19 +++++++++++++++++-- test/test_files/tinysnb/function/list.test | 6 ++++++ .../tinysnb/order_by/single_label.test | 18 +++++++++--------- 6 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/binder/bind/bind_projection_clause.cpp b/src/binder/bind/bind_projection_clause.cpp index fb845fcebb5..1205830faf8 100644 --- a/src/binder/bind/bind_projection_clause.cpp +++ b/src/binder/bind/bind_projection_clause.cpp @@ -113,7 +113,6 @@ void Binder::bindOrderBySkipLimitIfNecessary( // reference expression. auto projectionExpressionSet = expression_set{projectionExpressions.begin(), projectionExpressions.end()}; - for (auto& orderByExpression : orderByExpressions) { if (!projectionExpressionSet.contains(orderByExpression)) { throw BinderException("Order by expression " + orderByExpression->toString() + diff --git a/src/expression_evaluator/function_evaluator.cpp b/src/expression_evaluator/function_evaluator.cpp index 9546850ecab..d8f39aaa3d0 100644 --- a/src/expression_evaluator/function_evaluator.cpp +++ b/src/expression_evaluator/function_evaluator.cpp @@ -31,6 +31,19 @@ bool FunctionExpressionEvaluator::select(SelectionVector& selVector) { for (auto& child : children) { child->evaluate(); } + if (selectFunc == nullptr) { + assert(resultVector->dataType.typeID == BOOL); + execFunc(parameters, *resultVector); + auto numSelectedValues = 0u; + for (auto i = 0u; i < resultVector->state->selVector->selectedSize; ++i) { + auto pos = resultVector->state->selVector->selectedPositions[i]; + auto selectedPosBuffer = selVector.getSelectedPositionsBuffer(); + selectedPosBuffer[numSelectedValues] = pos; + numSelectedValues += resultVector->getValue(pos); + } + selVector.selectedSize = numSelectedValues; + return numSelectedValues > 0; + } return selectFunc(parameters, selVector); } diff --git a/src/include/planner/projection_planner.h b/src/include/planner/projection_planner.h index b8723851218..d01119e4c4c 100644 --- a/src/include/planner/projection_planner.h +++ b/src/include/planner/projection_planner.h @@ -20,6 +20,9 @@ class ProjectionPlanner { void planProjectionBody(const binder::BoundProjectionBody& projectionBody, LogicalPlan& plan); void planAggregate(const binder::expression_vector& expressionsToAggregate, const binder::expression_vector& expressionsToGroupBy, LogicalPlan& plan); + void planOrderBy(const binder::expression_vector& expressionsToProject, + const binder::expression_vector& expressionsToOrderBy, const std::vector& isAscOrders, + LogicalPlan& plan); void appendProjection(const binder::expression_vector& expressionsToProject, LogicalPlan& plan); void appendAggregate(const binder::expression_vector& expressionsToGroupBy, diff --git a/src/planner/projection_planner.cpp b/src/planner/projection_planner.cpp index 44d2c7ecc58..3621b3da8a3 100644 --- a/src/planner/projection_planner.cpp +++ b/src/planner/projection_planner.cpp @@ -51,8 +51,8 @@ void ProjectionPlanner::planProjectionBody( planAggregate(expressionsToAggregate, expressionsToGroupBy, plan); } if (projectionBody.hasOrderByExpressions()) { - appendOrderBy( - projectionBody.getOrderByExpressions(), projectionBody.getSortingOrders(), plan); + planOrderBy(expressionsToProject, projectionBody.getOrderByExpressions(), + projectionBody.getSortingOrders(), plan); } appendProjection(expressionsToProject, plan); if (projectionBody.getIsDistinct()) { @@ -86,6 +86,21 @@ void ProjectionPlanner::planAggregate(const expression_vector& expressionsToAggr appendAggregate(expressionsToGroupBy, expressionsToAggregate, plan); } +void ProjectionPlanner::planOrderBy(const binder::expression_vector& expressionsToProject, + const binder::expression_vector& expressionsToOrderBy, const std::vector& isAscOrders, + kuzu::planner::LogicalPlan& plan) { + auto expressionsToProjectBeforeOrderBy = expressionsToProject; + auto expressionsToProjectSet = + expression_set{expressionsToProject.begin(), expressionsToProject.end()}; + for (auto& expression : expressionsToOrderBy) { + if (!expressionsToProjectSet.contains(expression)) { + expressionsToProjectBeforeOrderBy.push_back(expression); + } + } + appendProjection(expressionsToProjectBeforeOrderBy, plan); + appendOrderBy(expressionsToOrderBy, isAscOrders, plan); +} + void ProjectionPlanner::appendProjection( const expression_vector& expressionsToProject, LogicalPlan& plan) { for (auto& expression : expressionsToProject) { diff --git a/test/test_files/tinysnb/function/list.test b/test/test_files/tinysnb/function/list.test index e5480f4908d..c89ecaebe23 100644 --- a/test/test_files/tinysnb/function/list.test +++ b/test/test_files/tinysnb/function/list.test @@ -578,6 +578,12 @@ True False True +-NAME ListContainsSelect +-QUERY MATCH (a:person) WHERE list_contains(a.courseScoresPerTerm, [8]) RETURN a.ID +---- 2 +7 +8 + -NAME ListContainsStructuredListOfStrings -QUERY MATCH (a:person) RETURN list_has(a.usedNames, "Grad") ---- 8 diff --git a/test/test_files/tinysnb/order_by/single_label.test b/test/test_files/tinysnb/order_by/single_label.test index 051434d0c92..aa2973b4302 100644 --- a/test/test_files/tinysnb/order_by/single_label.test +++ b/test/test_files/tinysnb/order_by/single_label.test @@ -1,15 +1,15 @@ -NAME OrderByInt64Test --QUERY MATCH (p:person) RETURN p.age ORDER BY p.age +-QUERY MATCH (p:person) RETURN p.ID, p.age ORDER BY p.age + p.ID -PARALLELISM 3 ---- 8 -20 -20 -25 -30 -35 -40 -45 -83 +5|20 +7|20 +2|30 +8|25 +0|35 +3|45 +9|40 +10|83 -NAME OrderByInt32Test -QUERY MATCH (m:movies) RETURN m.length ORDER BY m.length