Minor optimize for go n steps (#4566)

* Minor fixes and improvements * fix ctest * fix rewriteVertexEdge2EdgeProp * fix fix
vesoft-inc · Sep 1, 2022 · 51d84a4 · 51d84a4
1 parent a379339
commit 51d84a4
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: