From dfae2d6abbb2f64abfeaf98f3baed75b6f3a8884 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Mon, 15 May 2023 11:39:50 +0800 Subject: [PATCH 1/2] m2n simple case --- src/graph/planner/ngql/GoPlanner.cpp | 16 ++++++++++------ src/graph/validator/GoValidator.cpp | 3 +-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/graph/planner/ngql/GoPlanner.cpp b/src/graph/planner/ngql/GoPlanner.cpp index e3b1c5dc8e8..e15293547f5 100644 --- a/src/graph/planner/ngql/GoPlanner.cpp +++ b/src/graph/planner/ngql/GoPlanner.cpp @@ -295,12 +295,16 @@ StatusOr GoPlanner::transform(AstContext* astCtx) { for (auto node : varPtr->writtenBy) { preRootNode_ = node; } - - auto argNode = Argument::make(qctx, from.runtimeVidName); - argNode->setColNames({from.runtimeVidName}); - argNode->setInputVertexRequired(false); - goCtx_->vidsVar = argNode->outputVar(); - startNode_ = argNode; + if (goCtx_->isSimple) { + startNode_ = preRootNode_; + goCtx_->vidsVar = varName; + } else { + auto argNode = Argument::make(qctx, from.runtimeVidName); + argNode->setColNames({from.runtimeVidName}); + argNode->setInputVertexRequired(false); + goCtx_->vidsVar = argNode->outputVar(); + startNode_ = argNode; + } } auto& steps = goCtx_->steps; if (!steps.isMToN() && steps.steps() == 0) { diff --git a/src/graph/validator/GoValidator.cpp b/src/graph/validator/GoValidator.cpp index e2b8da5923e..518a0179495 100644 --- a/src/graph/validator/GoValidator.cpp +++ b/src/graph/validator/GoValidator.cpp @@ -284,8 +284,7 @@ bool GoValidator::checkDstPropOrVertexExist(const Expression* expr) { } bool GoValidator::isSimpleCase() { - if (!goCtx_->limits.empty() || !goCtx_->distinct || goCtx_->filter || - goCtx_->from.fromType != FromType::kInstantExpr) { + if (!goCtx_->limits.empty() || !goCtx_->distinct || goCtx_->filter != nullptr) { return false; } // Check if the yield cluase uses: From a20fbc5f4270501152c6d0c9f8701fbbeb599ed4 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Mon, 15 May 2023 15:37:02 +0800 Subject: [PATCH 2/2] add test case --- src/graph/planner/ngql/GoPlanner.cpp | 3 + tests/tck/features/go/SimpleCase.feature | 91 +++++++++++++----------- 2 files changed, 53 insertions(+), 41 deletions(-) diff --git a/src/graph/planner/ngql/GoPlanner.cpp b/src/graph/planner/ngql/GoPlanner.cpp index e15293547f5..3f9b36e665d 100644 --- a/src/graph/planner/ngql/GoPlanner.cpp +++ b/src/graph/planner/ngql/GoPlanner.cpp @@ -296,6 +296,9 @@ StatusOr GoPlanner::transform(AstContext* astCtx) { preRootNode_ = node; } if (goCtx_->isSimple) { + // go from 'xxx' over edge yield distinct edge._dst as id | + // go from $-.id over edge yield distinct edge._dst + // above scenario, the second statement does not need the argument operator startNode_ = preRootNode_; goCtx_->vidsVar = varName; } else { diff --git a/tests/tck/features/go/SimpleCase.feature b/tests/tck/features/go/SimpleCase.feature index addcb1107ba..1e4246fb97d 100644 --- a/tests/tck/features/go/SimpleCase.feature +++ b/tests/tck/features/go/SimpleCase.feature @@ -366,23 +366,20 @@ Feature: Simple case | count(*) | | 28 | And the execution plan should be: - | id | name | dependencies | operator info | - | 17 | Aggregate | 15 | | - | 15 | Minus | 13,14 | | - | 13 | Project | 16 | | - | 16 | PassThrough | 12 | | - | 12 | Dedup | 11 | | - | 11 | Project | 10 | | - | 10 | HashInnerJoin | 5,9 | | - | 5 | Project | 4 | | - | 4 | Dedup | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 0 | | - | 0 | Start | | | - | 9 | ExpandAll | 8 | | - | 8 | Expand | 7 | | - | 7 | Argument | | | - | 14 | Project | 16 | | + | id | name | dependencies | operator info | + | 14 | Aggregate | 12 | | + | 12 | Minus | 10,11 | | + | 10 | Project | 13 | | + | 13 | PassThrough | 9 | | + | 9 | Project | 8 | | + | 8 | Dedup | 7 | | + | 7 | Expand | 5 | | + | 5 | Project | 4 | | + | 4 | Dedup | 3 | | + | 3 | ExpandAll | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 11 | Project | 13 | | Scenario: other simple case When profiling query: @@ -393,18 +390,15 @@ Feature: Simple case | count(*) | | 0 | And the execution plan should be: - | id | name | dependencies | operator info | - | 12 | Aggregate | 11 | | - | 11 | Dedup | 10 | | - | 10 | Project | 9 | | - | 9 | HashInnerJoin | 4,8 | | - | 4 | Project | 3 | | - | 3 | Dedup | 2 | | - | 2 | Expand | 0 | | - | 0 | Start | | | - | 8 | ExpandAll | 7 | | - | 7 | Expand | 6 | | - | 6 | Argument | | | + | id | name | dependencies | operator info | + | 9 | Aggregate | 8 | | + | 8 | Project | 7 | | + | 7 | Dedup | 6 | | + | 6 | Expand | 4 | | + | 4 | Project | 3 | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | 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(*) @@ -413,18 +407,33 @@ Feature: Simple case | COUNT(*) | | 22 | And the execution plan should be: - | id | name | dependencies | operator info | - | 12 | Aggregate | 11 | | - | 11 | Dedup | 10 | | - | 10 | Project | 9 | | - | 9 | HashInnerJoin | 4,8 | | - | 4 | Project | 3 | | - | 3 | Dedup | 2 | | - | 2 | Expand | 0 | | - | 0 | Start | | | - | 8 | ExpandAll | 7 | | - | 7 | Expand | 6 | | - | 6 | Argument | | | + | id | name | dependencies | operator info | + | 9 | Aggregate | 8 | | + | 8 | Project | 7 | | + | 7 | Dedup | 6 | | + | 6 | Expand | 4 | | + | 4 | Project | 3 | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + When profiling query: + """ + GO 1 STEP FROM "Tony Parker" OVER * YIELD distinct id($$) as id| GO 2 to 4 STEP FROM $-.id OVER * YIELD distinct id($$) | YIELD COUNT(*) + """ + Then the result should be, in any order, with relax comparison: + | COUNT(*) | + | 26 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 10 | Aggregate | 9 | | + | 9 | Project | 8 | | + | 8 | Dedup | 7 | | + | 7 | ExpandAll | 6 | | + | 6 | Expand | 4 | | + | 4 | Project | 3 | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | Scenario: could not be optimied cases When profiling query: