Remove useless inner join for go in simple case (#4556)

vesoft-inc · Aug 19, 2022 · d0e289c · d0e289c
1 parent 2b1b733
commit d0e289c
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 6 deletions.
diff --git a/src/graph/executor/query/GetDstBySrcExecutor.cpp b/src/graph/executor/query/GetDstBySrcExecutor.cpp
@@ -26,6 +26,7 @@ folly::Future<Status> GetDstBySrcExecutor::execute() {
   auto reqList = std::move(res).value();
   if (reqList.empty()) {
     DataSet emptyResult;
+    emptyResult.colNames = gd_->colNames();
     return finish(ResultBuilder()
                       .value(Value(std::move(emptyResult)))
                       .iter(Iterator::Kind::kSequential)

diff --git a/src/graph/planner/ngql/GoPlanner.cpp b/src/graph/planner/ngql/GoPlanner.cpp
@@ -597,6 +597,10 @@ StatusOr<SubPlan> GoPlanner::transform(AstContext* astCtx) {
   goCtx_->joinInput = goCtx_->from.fromType != FromType::kInstantExpr;
   goCtx_->joinDst = !goCtx_->exprProps.dstTagProps().empty();
   goCtx_->isSimple = isSimpleCase();
+  if (goCtx_->isSimple) {
+    // We don't need to do a inner join in such case.
+    goCtx_->joinInput = false;
+  }
   if (goCtx_->isSimple) {
     goCtx_->yieldExpr->columns()[0]->setExpr(InputPropertyExpression::make(qctx->objPool(), kDst));
   }
@@ -623,18 +627,32 @@ StatusOr<SubPlan> GoPlanner::transform(AstContext* astCtx) {
 }
 
 bool GoPlanner::isSimpleCase() {
-  if (goCtx_->joinInput || goCtx_->joinDst || goCtx_->filter || !goCtx_->distinct) {
+  if (goCtx_->joinDst || goCtx_->filter || !goCtx_->distinct) {
     return false;
   }
-  if (goCtx_->yieldExpr->columns().size() != 1 ||
-      goCtx_->yieldExpr->columns()[0]->expr()->kind() != Expression::Kind::kEdgeDst) {
-    return false;
+  auto& exprProps = goCtx_->exprProps;
+  if (!exprProps.srcTagProps().empty()) return false;
+  if (!exprProps.dstTagProps().empty()) return false;
+  for (auto& edgeProp : exprProps.edgeProps()) {
+    auto props = edgeProp.second;
+    if (props.size() != 1) return false;
+    if (props.find(kDst) == props.end()) return false;
   }
 
-  auto expr = static_cast<const EdgeDstIdExpression*>(goCtx_->yieldExpr->columns()[0]->expr());
-  if (expr->sym() != "*" && goCtx_->over.edgeTypes.size() != 1) {
+  if (goCtx_->yieldExpr->columns().size() != 1) {
     return false;
   }
+  if (goCtx_->yieldExpr->columns()[0]->expr()->kind() != Expression::Kind::kEdgeDst &&
+      goCtx_->yieldExpr->columns()[0]->expr()->kind() != Expression::Kind::kVarProperty) {
+    return false;
+  }
+  auto* expr = goCtx_->yieldExpr->columns()[0]->expr();
+  if (expr->kind() == Expression::Kind::kEdgeDst) {
+    auto dstExpr = static_cast<const EdgeDstIdExpression*>(expr);
+    if (dstExpr->sym() != "*" && goCtx_->over.edgeTypes.size() != 1) {
+      return false;
+    }
+  }
   return true;
 }
 

diff --git a/tests/tck/features/go/SimpleCase.feature b/tests/tck/features/go/SimpleCase.feature
@@ -66,3 +66,39 @@ Feature: Simple case
     Then the result should be, in any order, with relax comparison:
       | count(*) |
       | 28       |
+    And the execution plan should be:
+      | id | name        | dependencies | operator info     |
+      | 14 | Aggregate   | 12           |                   |
+      | 12 | Minus       | 10,11        |                   |
+      | 10 | Project     | 13           |                   |
+      | 13 | PassThrough | 9            |                   |
+      | 9  | Dedup       | 8            |                   |
+      | 8  | GetDstBySrc | 7            |                   |
+      | 7  | Dedup       | 6            |                   |
+      | 6  | Project     | 5            |                   |
+      | 5  | DataCollect | 4            |                   |
+      | 4  | Loop        | 0            | {"loopBody": "3"} |
+      | 3  | Dedup       | 2            |                   |
+      | 2  | GetDstBySrc | 1            |                   |
+      | 1  | Start       |              |                   |
+      | 0  | Start       |              |                   |
+      | 11 | Project     | 13           |                   |
+
+  Scenario: other simple case
+    When profiling query:
+      """
+      GO FROM "Tony Parker" OVER serve BIDIRECT YIELD DISTINCT id($$) as dst | GO FROM $-.dst OVER serve YIELD DISTINCT id($$) as dst | YIELD count(*)
+      """
+    Then the result should be, in any order, with relax comparison:
+      | count(*) |
+      | 0        |
+    And the execution plan should be:
+      | id | name        | dependencies | operator info |
+      | 7  | Aggregate   | 6            |               |
+      | 6  | Dedup       | 5            |               |
+      | 5  | GetDstBySrc | 4            |               |
+      | 4  | Dedup       | 3            |               |
+      | 3  | Project     | 2            |               |
+      | 2  | Dedup       | 1            |               |
+      | 1  | GetDstBySrc | 0            |               |
+      | 0  | Start       |              |               |