From b15aeb04233ae00700fee3cf499d0e699009ac7e Mon Sep 17 00:00:00 2001 From: jimingquan Date: Thu, 27 Apr 2023 15:23:42 +0800 Subject: [PATCH] add go simple plan --- src/graph/context/ast/QueryAstContext.h | 1 + src/graph/executor/query/ExpandExecutor.cpp | 3 +- src/graph/planner/ngql/GoPlanner.cpp | 30 +++++ src/graph/planner/ngql/GoPlanner.h | 2 + src/graph/validator/GoValidator.cpp | 38 +++++++ src/graph/validator/GoValidator.h | 2 + tests/tck/features/go/SimpleCase.feature | 104 +++++++++--------- .../PushFilterDownHashLeftJoinRule.feature | 29 +++-- 8 files changed, 138 insertions(+), 71 deletions(-) diff --git a/src/graph/context/ast/QueryAstContext.h b/src/graph/context/ast/QueryAstContext.h index 5e8d60fdcda..db981658b82 100644 --- a/src/graph/context/ast/QueryAstContext.h +++ b/src/graph/context/ast/QueryAstContext.h @@ -93,6 +93,7 @@ struct GoContext final : AstContext { bool joinInput{false}; // true when $$.tag.prop exist bool joinDst{false}; + bool isSimple{false}; ExpressionProps exprProps; diff --git a/src/graph/executor/query/ExpandExecutor.cpp b/src/graph/executor/query/ExpandExecutor.cpp index 9cc1be79dc4..e5a1d9f295d 100644 --- a/src/graph/executor/query/ExpandExecutor.cpp +++ b/src/graph/executor/query/ExpandExecutor.cpp @@ -94,7 +94,8 @@ folly::Future ExpandExecutor::GetDstBySrc() { size = (*result.dsts_ref()).size(); } auto info = util::collectRespProfileData(result.result, hostLatency[i], size); - otherStats_.emplace(folly::sformat("resp[{}]", i), folly::toPrettyJson(info)); + otherStats_.emplace(folly::sformat("step{} resp [{}]", currentStep_, i), + folly::toPrettyJson(info)); } auto result = handleCompleteness(resps, FLAGS_accept_partial_success); if (!result.ok()) { diff --git a/src/graph/planner/ngql/GoPlanner.cpp b/src/graph/planner/ngql/GoPlanner.cpp index 4721c1e8d3a..0516cf6788a 100644 --- a/src/graph/planner/ngql/GoPlanner.cpp +++ b/src/graph/planner/ngql/GoPlanner.cpp @@ -142,6 +142,33 @@ PlanNode* GoPlanner::buildJoinDstPlan(PlanNode* dep) { return join; } +SubPlan GoPlanner::doSimplePlan() { + auto qctx = goCtx_->qctx; + size_t step = goCtx_->steps.mSteps(); + auto* expand = Expand::make(qctx, + startNode_, + goCtx_->space.id, + false, // random + step, + buildEdgeProps(true)); + expand->setEdgeTypes(buildEdgeTypes()); + expand->setColNames({"_expand_vid"}); + expand->setInputVar(goCtx_->vidsVar); + + auto* dedup = Dedup::make(qctx, expand); + + auto pool = qctx->objPool(); + auto* newYieldExpr = pool->makeAndAdd(); + newYieldExpr->addColumn(new YieldColumn(ColumnExpression::make(pool, 0))); + auto* project = Project::make(qctx, dedup, newYieldExpr); + project->setColNames(std::move(goCtx_->colNames)); + + SubPlan subPlan; + subPlan.root = project; + subPlan.tail = expand; + return subPlan; +} + SubPlan GoPlanner::doPlan() { auto qctx = goCtx_->qctx; auto& from = goCtx_->from; @@ -259,6 +286,9 @@ StatusOr GoPlanner::transform(AstContext* astCtx) { subPlan.root = subPlan.tail = pt; return subPlan; } + if (goCtx_->isSimple) { + return doSimplePlan(); + } return doPlan(); } diff --git a/src/graph/planner/ngql/GoPlanner.h b/src/graph/planner/ngql/GoPlanner.h index 9f620778fb1..1a430529039 100644 --- a/src/graph/planner/ngql/GoPlanner.h +++ b/src/graph/planner/ngql/GoPlanner.h @@ -34,6 +34,8 @@ class GoPlanner final : public Planner { private: SubPlan doPlan(); + SubPlan doSimplePlan(); + private: std::unique_ptr buildVertexProps(const ExpressionProps::TagIDPropsMap& propsMap); diff --git a/src/graph/validator/GoValidator.cpp b/src/graph/validator/GoValidator.cpp index 3a0b80d57e5..b280721beb7 100644 --- a/src/graph/validator/GoValidator.cpp +++ b/src/graph/validator/GoValidator.cpp @@ -56,6 +56,7 @@ Status GoValidator::validateImpl() { exprProps.varProps().size() > 1) { return Status::SemanticError("Only support single input in a go sentence."); } + goCtx_->isSimple = isSimpleCase(); NG_RETURN_IF_ERROR(buildColumns()); return Status::OK(); @@ -282,5 +283,42 @@ bool GoValidator::checkDstPropOrVertexExist(const Expression* expr) { return true; } +bool GoValidator::isSimpleCase() { + if (!goCtx_->limits.empty() || !goCtx_->distinct || goCtx_->filter || goCtx_->steps.isMToN() || + goCtx_->from.fromType != FromType::kInstantExpr) { + return false; + } + // Check if the yield cluase uses: + // 1. src tag props, + // 2. or edge props, except the dst id of edge. + // 3. input or var props. + auto& exprProps = goCtx_->exprProps; + if (!exprProps.srcTagProps().empty() || !exprProps.dstTagProps().empty()) { + return false; + } + if (!exprProps.edgeProps().empty()) { + for (auto& edgeProp : exprProps.edgeProps()) { + auto props = edgeProp.second; + if (props.size() != 1 && props.find(kDst) == props.end()) { + return false; + } + } + } + + bool atLeastOneDstId = false; + for (auto& col : goCtx_->yieldExpr->columns()) { + auto expr = col->expr(); + if (expr->kind() != Expression::Kind::kEdgeDst) { + return false; + } + atLeastOneDstId = true; + auto dstIdExpr = static_cast(expr); + if (dstIdExpr->sym() != "*" && goCtx_->over.edgeTypes.size() != 1) { + return false; + } + } + return atLeastOneDstId; +} + } // namespace graph } // namespace nebula diff --git a/src/graph/validator/GoValidator.h b/src/graph/validator/GoValidator.h index 241f88c4a43..5a7a42f9fb9 100644 --- a/src/graph/validator/GoValidator.h +++ b/src/graph/validator/GoValidator.h @@ -44,6 +44,8 @@ class GoValidator final : public Validator { bool checkDstPropOrVertexExist(const Expression* expr); + bool isSimpleCase(); + private: std::unique_ptr goCtx_; diff --git a/tests/tck/features/go/SimpleCase.feature b/tests/tck/features/go/SimpleCase.feature index 8a5bf1f8ba7..625670d2899 100644 --- a/tests/tck/features/go/SimpleCase.feature +++ b/tests/tck/features/go/SimpleCase.feature @@ -16,12 +16,11 @@ Feature: Simple case | 2 | And the execution plan should be: | id | name | dependencies | operator info | - | 6 | Aggregate | 5 | | - | 5 | Dedup | 4 | | + | 5 | Aggregate | 4 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | When profiling query: """ GO FROM "Yao Ming" OVER like YIELD DISTINCT id($$) AS dst, $$.player.age AS age | ORDER BY $-.dst @@ -72,13 +71,12 @@ Feature: Simple case | "Manu Ginobili" | | "Tim Duncan" | And the execution plan should be: - | id | name | dependencies | operator info | - | 6 | Sort | 5 | | - | 5 | Dedup | 4 | | - | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | + | id | name | dependencies | operator info | + | 5 | Sort | 4 | | + | 4 | Project | 3 | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | When profiling query: """ GO FROM "Tony Parker" OVER like YIELD DISTINCT 2, id($$) AS a | ORDER BY $-.a @@ -107,12 +105,11 @@ Feature: Simple case | 22 | And the execution plan should be: | id | name | dependencies | operator info | - | 6 | Aggregate | 5 | | - | 5 | Dedup | 4 | | + | 5 | Aggregate | 4 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | When profiling query: """ GO 3 STEPS FROM "Tony Parker" OVER serve BIDIRECT WHERE $$.team.name != "Lakers" YIELD DISTINCT id($$) | YIELD count(*) @@ -397,18 +394,17 @@ Feature: Simple case | 0 | And the execution plan should be: | id | name | dependencies | operator info | - | 13 | Aggregate | 12 | | - | 12 | Dedup | 11 | | - | 11 | Project | 10 | | - | 10 | HashInnerJoin | 5,9 | | - | 5 | Dedup | 4 | | + | 12 | Aggregate | 11 | | + | 11 | Dedup | 10 | | + | 10 | Project | 9 | | + | 9 | HashInnerJoin | 4,8 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | - | 9 | ExpandAll | 8 | | - | 8 | Expand | 7 | | - | 7 | Argument | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 8 | ExpandAll | 7 | | + | 7 | Expand | 6 | | + | 6 | Argument | | | When profiling query: """ GO 1 STEP FROM "Tony Parker" OVER * YIELD distinct id($$) as id| GO 3 STEP FROM $-.id OVER * YIELD distinct id($$) | YIELD COUNT(*) @@ -418,18 +414,17 @@ Feature: Simple case | 22 | And the execution plan should be: | id | name | dependencies | operator info | - | 13 | Aggregate | 12 | | - | 12 | Dedup | 11 | | - | 11 | Project | 10 | | - | 10 | HashInnerJoin | 5,9 | | - | 5 | Dedup | 4 | | + | 12 | Aggregate | 11 | | + | 11 | Dedup | 10 | | + | 10 | Project | 9 | | + | 9 | HashInnerJoin | 4,8 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | - | 9 | ExpandAll | 8 | | - | 8 | Expand | 7 | | - | 7 | Argument | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 8 | ExpandAll | 7 | | + | 7 | Expand | 6 | | + | 6 | Argument | | | Scenario: could not be optimied cases When profiling query: @@ -584,20 +579,19 @@ Feature: Simple case | "Grant Hill" | 46 | "Grant Hill" | And the execution plan should be: | id | name | dependencies | operator info | - | 18 | Sort | 17 | | - | 17 | Dedup | 16 | | - | 16 | Project | 21 | | - | 21 | HashInnerJoin | 5,20 | | - | 5 | Dedup | 4 | | + | 17 | Sort | 16 | | + | 16 | Dedup | 15 | | + | 15 | Project | 20 | | + | 20 | HashInnerJoin | 4,19 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | - | 20 | Filter | 19 | | - | 19 | HashLeftJoin | 9,12 | | - | 9 | ExpandAll | 8 | | - | 8 | Expand | 7 | | - | 7 | Argument | | | - | 12 | Project | 11 | | - | 11 | GetVertices | 10 | | - | 10 | Argument | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 19 | Filter | 18 | | + | 18 | HashLeftJoin | 8,11 | | + | 8 | ExpandAll | 7 | | + | 7 | Expand | 6 | | + | 6 | Argument | | | + | 11 | Project | 10 | | + | 10 | GetVertices | 9 | | + | 9 | Argument | | | diff --git a/tests/tck/features/optimizer/PushFilterDownHashLeftJoinRule.feature b/tests/tck/features/optimizer/PushFilterDownHashLeftJoinRule.feature index c5a7ef3995d..c5a20a0a4a1 100644 --- a/tests/tck/features/optimizer/PushFilterDownHashLeftJoinRule.feature +++ b/tests/tck/features/optimizer/PushFilterDownHashLeftJoinRule.feature @@ -63,22 +63,21 @@ Feature: Push Filter down HashLeftJoin rule | "Boris Diaw" | "Suns" | ["team"] | And the execution plan should be: | id | name | dependencies | operator info | - | 20 | TopN | 17 | | - | 17 | Dedup | 16 | | - | 16 | Project | 23 | | - | 23 | HashInnerJoin | 5,26 | | - | 5 | Dedup | 4 | | + | 19 | TopN | 16 | | + | 16 | Dedup | 15 | | + | 15 | Project | 22 | | + | 22 | HashInnerJoin | 4,25 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | - | 26 | HashLeftJoin | 27,12 | | - | 27 | ExpandAll | 8 | {"filter": "((like.likeness>80) OR like.likeness IS EMPTY)"} | - | 8 | Expand | 7 | | - | 7 | Argument | | | - | 12 | Project | 11 | | - | 11 | GetVertices | 10 | | - | 10 | Argument | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 25 | HashLeftJoin | 26,11 | | + | 26 | ExpandAll | 7 | {"filter": "((like.likeness>80) OR like.likeness IS EMPTY)"} | + | 7 | Expand | 6 | | + | 6 | Argument | | | + | 11 | Project | 10 | | + | 10 | GetVertices | 9 | | + | 9 | Argument | | | Scenario: NOT push filter down HashLeftJoin When profiling query: