Minor optimize for go n steps (vesoft-inc#4566)

* Minor fixes and improvements * fix ctest * fix rewriteVertexEdge2EdgeProp * fix fix
caton-hpg · Sep 1, 2022 · 952d1ba · 952d1ba
1 parent 8e43bfd
commit 952d1ba
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 17 deletions.
diff --git a/src/graph/planner/ngql/GoPlanner.cpp b/src/graph/planner/ngql/GoPlanner.cpp
@@ -439,12 +439,11 @@ SubPlan GoPlanner::oneStepPlan(SubPlan& startVidPlan) {
 
 SubPlan GoPlanner::nStepsPlan(SubPlan& startVidPlan) {
  auto qctx = goCtx_->qctx;
- auto isSimple = goCtx_->isSimple;
  loopStepVar_ = qctx->vctx()->anonVarGen()->getVar();
 
  auto* start = StartNode::make(qctx);
  PlanNode* scan = nullptr;
- if (isSimple) {
+ if (!goCtx_->joinInput && goCtx_->limits.empty()) {
  auto* gd = GetDstBySrc::make(qctx, start, goCtx_->space.id);
  gd->setSrc(goCtx_->from.src);
  gd->setEdgeTypes(buildEdgeTypes());
@@ -461,7 +460,7 @@ SubPlan GoPlanner::nStepsPlan(SubPlan& startVidPlan) {
  auto* sampleLimit = buildSampleLimit(scan);
 
  PlanNode* getDst = nullptr;
- if (isSimple) {
+ if (!goCtx_->joinInput && goCtx_->limits.empty()) {
  auto* dedup = Dedup::make(qctx, sampleLimit);
  dedup->setOutputVar(goCtx_->vidsVar);
  dedup->setColNames(goCtx_->colNames);
@@ -627,7 +626,7 @@ StatusOr<SubPlan> GoPlanner::transform(AstContext* astCtx) {
 }
 
 bool GoPlanner::isSimpleCase() {
- if (goCtx_->joinDst || goCtx_->filter || !goCtx_->distinct) {
+ if (goCtx_->joinDst || goCtx_->filter || !goCtx_->distinct || !goCtx_->limits.empty()) {
  return false;
  }
  auto& exprProps = goCtx_->exprProps;

diff --git a/src/graph/validator/test/QueryValidatorTest.cpp b/src/graph/validator/test/QueryValidatorTest.cpp
@@ -81,8 +81,7 @@ TEST_F(QueryValidatorTest, GoNSteps) {
  PK::kLoop,
  PK::kStart,
  PK::kDedup,
- PK::kProject,
- PK::kGetNeighbors,
+ PK::kGetDstBySrc,
  PK::kStart};
  EXPECT_TRUE(checkResult(query, expected));
  }
@@ -96,8 +95,7 @@ TEST_F(QueryValidatorTest, GoNSteps) {
  PK::kLoop,
  PK::kStart,
  PK::kDedup,
- PK::kProject,
- PK::kGetNeighbors,
+ PK::kGetDstBySrc,
  PK::kStart,
  };
  EXPECT_TRUE(checkResult(query, expected));
@@ -114,8 +112,7 @@ TEST_F(QueryValidatorTest, GoNSteps) {
  PK::kLoop,
  PK::kStart,
  PK::kDedup,
- PK::kProject,
- PK::kGetNeighbors,
+ PK::kGetDstBySrc,
  PK::kStart,
  };
  EXPECT_TRUE(checkResult(query, expected));
@@ -131,8 +128,7 @@ TEST_F(QueryValidatorTest, GoNSteps) {
  PK::kLoop,
  PK::kStart,
  PK::kDedup,
- PK::kProject,
- PK::kGetNeighbors,
+ PK::kGetDstBySrc,
  PK::kStart};
  EXPECT_TRUE(checkResult(query, expected));
  }
@@ -166,8 +162,7 @@ TEST_F(QueryValidatorTest, GoWithPipe) {
  PK::kLoop,
  PK::kStart,
  PK::kDedup,
- PK::kProject,
- PK::kGetNeighbors,
+ PK::kGetDstBySrc,
  PK::kStart};
  EXPECT_TRUE(checkResult(query, expected));
  }
@@ -459,8 +454,7 @@ TEST_F(QueryValidatorTest, GoReversely) {
  PK::kLoop,
  PK::kStart,
  PK::kDedup,
- PK::kProject,
- PK::kGetNeighbors,
+ PK::kGetDstBySrc,
  PK::kStart,
  };
  EXPECT_TRUE(checkResult(query, expected));

diff --git a/tests/tck/features/go/SimpleCase.feature b/tests/tck/features/go/SimpleCase.feature
@@ -21,7 +21,7 @@ Feature: Simple case
  | 1 | GetDstBySrc | 0 | |
  | 0 | Start | | |
 
- Scenario: go m steps yield distinct dst id
+ Scenario: go m steps
  When profiling query:
  """
  GO 3 STEPS FROM "Tony Parker" OVER serve BIDIRECT YIELD DISTINCT id($$) AS dst | YIELD count(*)
@@ -39,6 +39,89 @@ Feature: Simple case
  | 2 | GetDstBySrc | 1 | |
  | 1 | Start | | |
  | 0 | Start | | |
+ # The last step degenerates to `GetNeighbors` when yield or filter properties other than `_dst`
+ When profiling query:
+ """
+ GO 3 STEPS FROM "Tony Parker" OVER serve BIDIRECT YIELD id($$) AS dst | YIELD count(*)
+ """
+ Then the result should be, in any order, with relax comparison:
+ | count(*) |
+ | 65 |
+ And the execution plan should be:
+ | id | name | dependencies | operator info |
+ | 7 | Aggregate | 6 | |
+ | 6 | Project | 5 | |
+ | 5 | GetNeighbors | 4 | |
+ | 4 | Loop | 0 | {"loopBody": "3"} |
+ | 3 | Dedup | 2 | |
+ | 2 | GetDstBySrc | 1 | |
+ | 1 | Start | | |
+ | 0 | Start | | |
+ When profiling query:
+ """
+ GO 3 STEPS FROM "Tony Parker" OVER serve BIDIRECT YIELD $$.player.age AS age | YIELD count(*)
+ """
+ Then the result should be, in any order, with relax comparison:
+ | count(*) |
+ | 65 |
+ And the execution plan should be:
+ | id | name | dependencies | operator info |
+ | 11 | Aggregate | 10 | |
+ | 10 | Project | 9 | |
+ | 9 | LeftJoin | 8 | |
+ | 8 | Project | 7 | |
+ | 7 | GetVertices | 6 | |
+ | 6 | Project | 5 | |
+ | 5 | GetNeighbors | 4 | |
+ | 4 | Loop | 0 | {"loopBody": "3"} |
+ | 3 | Dedup | 2 | |
+ | 2 | GetDstBySrc | 1 | |
+ | 1 | Start | | |
+ | 0 | Start | | |
+ When profiling query:
+ """
+ GO 3 STEPS FROM "Tony Parker" OVER * WHERE $$.player.age > 36 YIELD $$.player.age AS age | YIELD count(*)
+ """
+ Then the result should be, in any order, with relax comparison:
+ | count(*) |
+ | 10 |
+ And the execution plan should be:
+ | id | name | dependencies | operator info |
+ | 12 | Aggregate | 11 | |
+ | 11 | Project | 10 | |
+ | 10 | Filter | 9 | |
+ | 9 | LeftJoin | 8 | |
+ | 8 | Project | 7 | |
+ | 7 | GetVertices | 6 | |
+ | 6 | Project | 5 | |
+ | 5 | GetNeighbors | 4 | |
+ | 4 | Loop | 0 | {"loopBody": "3"} |
+ | 3 | Dedup | 2 | |
+ | 2 | GetDstBySrc | 1 | |
+ | 1 | Start | | |
+ | 0 | Start | | |
+ # Because GetDstBySrc doesn't support limit push down, so degenrate to GetNeighbors when the query contains limit/simple clause
+ When profiling query:
+ """
+ GO 3 STEPS FROM "Tony Parker" OVER * YIELD DISTINCT id($$) LIMIT [100, 100, 100] | YIELD count(*)
+ """
+ Then the result should be, in any order, with relax comparison:
+ | count(*) |
+ | 13 |
+ And the execution plan should be:
+ | id | name | dependencies | operator info |
+ | 11 | Aggregate | 10 | |
+ | 10 | Dedup | 9 | |
+ | 9 | Project | 12 | |
+ | 12 | Limit | 13 | |
+ | 13 | GetNeighbors | 6 | |
+ | 6 | Loop | 0 | {"loopBody": "5"} |
+ | 5 | Dedup | 4 | |
+ | 4 | Project | 14 | |
+ | 14 | Limit | 15 | |
+ | 15 | GetNeighbors | 1 | |
+ | 1 | Start | | |
+ | 0 | Start | | |
 
  Scenario: go m to n steps yield distinct dst id
  When profiling query: