fix multiple match (#4143)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

src/graph/planner/match/MatchClausePlanner.cpp

@@ -148,7 +148,6 @@ StatusOr<SubPlan> MatchClausePlanner::transform(CypherClauseContextBase* clauseC
         connectPathPlan(nodeInfos, matchClauseCtx, subplan, nodeAliasesSeen, matchClausePlan));
   }
   NG_RETURN_IF_ERROR(projectColumnsBySymbols(matchClauseCtx, matchClausePlan));
-  NG_RETURN_IF_ERROR(appendFilterPlan(matchClauseCtx, matchClausePlan));
   return matchClausePlan;
 }
 

src/graph/planner/match/MatchPlanner.cpp

@@ -10,6 +10,7 @@
 #include "graph/planner/match/ReturnClausePlanner.h"
 #include "graph/planner/match/SegmentsConnector.h"
 #include "graph/planner/match/UnwindClausePlanner.h"
+#include "graph/planner/match/WhereClausePlanner.h"
 #include "graph/planner/match/WithClausePlanner.h"
 #include "graph/planner/plan/Algo.h"
 #include "graph/planner/plan/Logic.h"
@@ -28,14 +29,7 @@ StatusOr<SubPlan> MatchPlanner::transform(AstContext* astCtx) {
   auto* cypherCtx = static_cast<CypherContext*>(astCtx);
   SubPlan queryPlan;
   for (auto& queryPart : cypherCtx->queryParts) {
-    SubPlan queryPartPlan;
-    for (auto& match : queryPart.matchs) {
-      auto matchPlanStatus = genPlan(match.get());
-      NG_RETURN_IF_ERROR(matchPlanStatus);
-      auto matchPlan = std::move(matchPlanStatus).value();
-      connectMatch(match.get(), matchPlan, queryPartPlan);
-    }
-    NG_RETURN_IF_ERROR(connectQueryParts(queryPart, queryPartPlan, cypherCtx->qctx, queryPlan));
+    NG_RETURN_IF_ERROR(genQueryPartPlan(cypherCtx->qctx, queryPlan, queryPart));
   }
 
   return queryPlan;
@@ -62,89 +56,84 @@ StatusOr<SubPlan> MatchPlanner::genPlan(CypherClauseContextBase* clauseCtx) {
   return Status::OK();
 }
 
-void MatchPlanner::connectMatch(const MatchClauseContext* match,
-                                const SubPlan& matchPlan,
-                                SubPlan& queryPartPlan) {
-  if (queryPartPlan.root == nullptr) {
-    queryPartPlan = matchPlan;
-    return;
+// Connect current match plan to previous queryPlan
+Status MatchPlanner::connectMatchPlan(SubPlan& queryPlan, MatchClauseContext* matchCtx) {
+  // Generate current match plan
+  auto matchPlanStatus = genPlan(matchCtx);
+  NG_RETURN_IF_ERROR(matchPlanStatus);
+  auto matchPlan = std::move(matchPlanStatus).value();
+
+  if (queryPlan.root == nullptr) {
+    queryPlan = matchPlan;
+    return Status::OK();
   }
-  const auto& path = match->paths.back();
+  // Connect to queryPlan
+  const auto& path = matchCtx->paths.back();
   if (path.rollUpApply) {
-    queryPartPlan = SegmentsConnector::rollUpApply(
-        match->qctx, queryPartPlan, matchPlan, path.compareVariables, path.collectVariable);
-    return;
+    queryPlan = SegmentsConnector::rollUpApply(
+        matchCtx->qctx, queryPlan, matchPlan, path.compareVariables, path.collectVariable);
+    return Status::OK();
   }
+
   std::unordered_set<std::string> intersectedAliases;
-  for (auto& alias : match->aliasesGenerated) {
-    if (match->aliasesAvailable.find(alias.first) != match->aliasesAvailable.end()) {
-      intersectedAliases.emplace(alias.first);
+  for (auto& alias : matchCtx->aliasesGenerated) {
+    if (matchCtx->aliasesAvailable.find(alias.first) != matchCtx->aliasesAvailable.end()) {
+      intersectedAliases.insert(alias.first);
     }
   }
-
   if (!intersectedAliases.empty()) {
-    if (match->isOptional) {
-      queryPartPlan =
-          SegmentsConnector::leftJoin(match->qctx, queryPartPlan, matchPlan, intersectedAliases);
+    if (matchCtx->isOptional) {
+      queryPlan =
+          SegmentsConnector::leftJoin(matchCtx->qctx, matchPlan, queryPlan, intersectedAliases);
     } else {
-      queryPartPlan =
-          SegmentsConnector::innerJoin(match->qctx, queryPartPlan, matchPlan, intersectedAliases);
+      queryPlan =
+          SegmentsConnector::innerJoin(matchCtx->qctx, matchPlan, queryPlan, intersectedAliases);
     }
   } else {
-    queryPartPlan = SegmentsConnector::cartesianProduct(match->qctx, queryPartPlan, matchPlan);
+    queryPlan.root = BiCartesianProduct::make(matchCtx->qctx, queryPlan.root, matchPlan.root);
   }
+
+  return Status::OK();
 }
 
-Status MatchPlanner::connectQueryParts(const QueryPart& queryPart,
-                                       const SubPlan& partPlan,
-                                       QueryContext* qctx,
-                                       SubPlan& queryPlan) {
-  auto boundaryPlan = genPlan(queryPart.boundary.get());
-  NG_RETURN_IF_ERROR(boundaryPlan);
-  // If this is the first query part, there will be no CartesianProduct or Join
-  if (queryPlan.root == nullptr) {
-    auto subplan = std::move(boundaryPlan).value();
-    if (partPlan.root != nullptr) {
-      subplan = SegmentsConnector::addInput(subplan, partPlan);
-    }
-    queryPlan = subplan;
-    if (queryPlan.tail->isSingleInput()) {
-      queryPlan.tail->setInputVar(qctx->vctx()->anonVarGen()->getVar());
+Status MatchPlanner::genQueryPartPlan(QueryContext* qctx,
+                                      SubPlan& queryPlan,
+                                      const QueryPart& queryPart) {
+  // generate plan for matchs
+  for (auto& match : queryPart.matchs) {
+    connectMatchPlan(queryPlan, match.get());
+    // connect match filter
+    if (match->where != nullptr) {
+      auto wherePlanStatus = std::make_unique<WhereClausePlanner>()->transform(match->where.get());
+      NG_RETURN_IF_ERROR(wherePlanStatus);
+      auto wherePlan = std::move(wherePlanStatus).value();
+      queryPlan = SegmentsConnector::addInput(wherePlan, queryPlan, true);
     }
-    return Status::OK();
   }
 
-  // Otherwise, there might only a with/unwind/return in a query part
-  if (partPlan.root == nullptr) {
-    auto subplan = std::move(boundaryPlan).value();
-    queryPlan = SegmentsConnector::addInput(subplan, queryPlan);
-    return Status::OK();
-  }
-
-  auto& aliasesAvailable = queryPart.aliasesAvailable;
-  std::unordered_set<std::string> intersectedAliases;
-  auto& firstMatch = queryPart.matchs.front();
-  for (auto& alias : firstMatch->aliasesGenerated) {
-    if (aliasesAvailable.find(alias.first) != aliasesAvailable.end()) {
-      intersectedAliases.insert(alias.first);
-    }
+  // generate plan for boundary
+  auto boundaryPlanStatus = genPlan(queryPart.boundary.get());
+  NG_RETURN_IF_ERROR(boundaryPlanStatus);
+  auto boundaryPlan = std::move(boundaryPlanStatus).value();
+  if (queryPlan.root == nullptr) {
+    queryPlan = boundaryPlan;
+  } else {
+    queryPlan = SegmentsConnector::addInput(boundaryPlan, queryPlan, false);
   }
 
-  if (!intersectedAliases.empty()) {
-    if (firstMatch->isOptional) {
-      queryPlan =
-          SegmentsConnector::leftJoin(firstMatch->qctx, queryPlan, partPlan, intersectedAliases);
-    } else {
-      queryPlan =
-          SegmentsConnector::innerJoin(firstMatch->qctx, queryPlan, partPlan, intersectedAliases);
+  // TBD: need generate var for all queryPlan.tail?
+  if (queryPlan.tail->isSingleInput()) {
+    queryPlan.tail->setInputVar(qctx->vctx()->anonVarGen()->getVar());
+    if (!tailConnected_) {
+      auto start = StartNode::make(qctx);
+      queryPlan.tail->setDep(0, start);
+      tailConnected_ = true;
+      queryPlan.tail = start;
     }
-  } else {
-    queryPlan.root = BiCartesianProduct::make(qctx, queryPlan.root, partPlan.root);
   }
+  VLOG(1) << queryPlan;
 
-  queryPlan = SegmentsConnector::addInput(boundaryPlan.value(), queryPlan);
   return Status::OK();
 }
-
 }  // namespace graph
 }  // namespace nebula

src/graph/planner/match/MatchPlanner.h

@@ -23,16 +23,10 @@ class MatchPlanner final : public Planner {
   StatusOr<SubPlan> transform(AstContext* astCtx) override;
 
  private:
+  bool tailConnected_{false};
   StatusOr<SubPlan> genPlan(CypherClauseContextBase* clauseCtx);
-
-  void connectMatch(const MatchClauseContext* match,
-                    const SubPlan& matchPlan,
-                    SubPlan& queryPartPlan);
-
-  Status connectQueryParts(const QueryPart& queryPart,
-                           const SubPlan& partPlan,
-                           QueryContext* qctx,
-                           SubPlan& queryPlan);
+  Status connectMatchPlan(SubPlan& queryPlan, MatchClauseContext* matchCtx);
+  Status genQueryPartPlan(QueryContext* qctx, SubPlan& queryPlan, const QueryPart& queryPart);
 };
 }  // namespace graph
 }  // namespace nebula

src/graph/planner/plan/Logic.cpp

@@ -69,7 +69,8 @@ std::unique_ptr<PlanNodeDescription> Select::explain() const {
   return desc;
 }
 
-Argument::Argument(QueryContext* qctx, std::string alias) : PlanNode(qctx, Kind::kArgument) {
+Argument::Argument(QueryContext* qctx, std::string alias, const PlanNode* dep)
+    : SingleInputNode(qctx, Kind::kArgument, dep) {
   alias_ = alias;
   // An argument is a kind of leaf node, it has no dependencies but read a variable.
   inputVars_.emplace_back(nullptr);

src/graph/planner/plan/Logic.h

@@ -143,10 +143,10 @@ class PassThroughNode final : public SingleInputNode {
 };
 
 // This operator is used for getting a named alias from another executed operator.
-class Argument final : public PlanNode {
+class Argument final : public SingleInputNode {
  public:
-  static Argument* make(QueryContext* qctx, std::string alias) {
-    return qctx->objPool()->add(new Argument(qctx, alias));
+  static Argument* make(QueryContext* qctx, std::string alias, const PlanNode* dep = nullptr) {
+    return qctx->objPool()->add(new Argument(qctx, alias, dep));
   }
 
   PlanNode* clone() const override;
@@ -158,7 +158,7 @@ class Argument final : public PlanNode {
   std::unique_ptr<PlanNodeDescription> explain() const override;
 
  private:
-  Argument(QueryContext* qctx, std::string alias);
+  Argument(QueryContext* qctx, std::string alias, const PlanNode* dep = nullptr);
 
   void cloneMembers(const Argument&);
 

tests/tck/features/match/MultiLineMultiQueryParts.feature

@@ -87,6 +87,26 @@ Feature: Multi Line Multi Query Parts
       | "Tim Duncan" | "Boris Diaw"  | "Spurs"      |
       | "Tim Duncan" | "Boris Diaw"  | "Suns"       |
       | "Tim Duncan" | "Boris Diaw"  | "Tim Duncan" |
+    When executing query:
+      """
+      USE nba;
+      MATCH (m)-[]-(n),(n) WHERE id(m)=="Tim Duncan" and id(n)=="Tony Parker"
+      MATCH (n)-[]-(l) where n.player.age<m.player.age
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 64    |
+    When executing query:
+      """
+      USE nba;
+      MATCH (v:player) WHERE v.player.age>43
+      MATCH (n:player) WHERE v.player.age>40 and n.player.age>v.player.age
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 5     |
     When executing query:
       """
       USE nba;
@@ -161,6 +181,36 @@ Feature: Multi Line Multi Query Parts
       | "Tim Duncan" | "Manu Ginobili"     | NULL |
       | "Tim Duncan" | "Manu Ginobili"     | NULL |
       | "Tim Duncan" | "Manu Ginobili"     | NULL |
+    When executing query:
+      """
+      USE nba;
+      MATCH (m)-[]-(n),(n) WHERE id(m)=="Tim Duncan" and id(n)=="Tony Parker"
+      OPTIONAL MATCH (n)-[]-(l) where n.player.age < m.player.age
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 64    |
+    When executing query:
+      """
+      USE nba;
+      OPTIONAL match (v:player) WHERE v.player.age > 41
+      MATCH (v:player) WHERE v.player.age>40
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 7     |
+    When executing query:
+      """
+      USE nba;
+      OPTIONAL match (v:player) WHERE v.player.age>43
+      MATCH (n:player) WHERE n.player.age>40
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 32    |
 
   Scenario: Multi Line Multi Query Parts
     When executing query:
@@ -207,6 +257,69 @@ Feature: Multi Line Multi Query Parts
     Then the result should be, in order:
       | scount |
       | 270    |
+    When executing query:
+      """
+      USE nba;
+      MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan"
+      OPTIONAL MATCH (n)-->(v) WHERE v.player.age < m.player.age
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 45    |
+    When executing query:
+      """
+      USE nba;
+      MATCH (a:player{age:42}) WITH a
+      MATCH (b:player{age:40}) WHERE b.player.age<a.player.age
+      UNWIND [1,2,3] as l
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 12    |
+    When executing query:
+      """
+      USE nba;
+      MATCH (a:player{age:42}) WITH a
+      MATCH (b:player{age:40}) WITH a,b WHERE b.player.age<a.player.age
+      UNWIND [1,2,3] as l
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 12    |
+    When executing query:
+      """
+      USE nba;
+      OPTIONAL match (v:player) WHERE v.player.age>43 WITH v
+      MATCH (v:player) WHERE v.player.age>40  WITH v
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 4     |
+    When executing query:
+      """
+      USE nba;
+      OPTIONAL match (v:player) WHERE v.player.age>43 WITH v
+      MATCH (n:player) WHERE n.player.age>40  WITH v, n
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 32    |
+    When executing query:
+      """
+      USE nba;
+      MATCH (a:player{age:42}) WITH a
+      MATCH (b:player{age:40}) WHERE b.player.age<a.player.age
+      UNWIND [1,2,3] AS l WITH a,b,l WHERE b.player.age+1 < a.player.age
+      RETURN count(*) AS count
+      """
+    Then the result should be, in order:
+      | count |
+      | 12    |
 
   Scenario: Multi Line Some Erros
     When executing query: