Pattern expression supports reference local edge variable (#5424)

vesoft-inc · Mar 21, 2023 · 05ca117 · 05ca117
1 parent 5cb0bff
commit 05ca117
Show file tree

Hide file tree

Showing 8 changed files with 165 additions and 18 deletions.
diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp
@@ -432,6 +432,7 @@ std::unordered_map<std::string, std::vector<TypeSignature>> FunctionManager::typ
       TypeSignature({Value::Type::MAP}, Value::Type::DURATION)}},
     {"extract", {TypeSignature({Value::Type::STRING, Value::Type::STRING}, Value::Type::LIST)}},
     {"_nodeid", {TypeSignature({Value::Type::PATH, Value::Type::INT}, Value::Type::INT)}},
+    {"_edge", {TypeSignature({Value::Type::PATH, Value::Type::INT}, Value::Type::EDGE)}},
     {"json_extract",
      {TypeSignature({Value::Type::STRING}, Value::Type::MAP),
       TypeSignature({Value::Type::STRING}, Value::Type::NULLVALUE)}},
@@ -2850,6 +2851,39 @@ FunctionManager::FunctionManager() {
       }
     };
   }
+  {
+    auto &attr = functions_["_edge"];
+    attr.minArity_ = 2;
+    attr.maxArity_ = 2;
+    attr.isAlwaysPure_ = true;
+    attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isPath() || !args[1].get().isInt()) {
+        return Value::kNullBadType;
+      }
+
+      const auto &p = args[0].get().getPath();
+      const std::size_t edgeIndex = args[1].get().getInt();
+      if (edgeIndex < 0 || edgeIndex >= p.steps.size()) {
+        DLOG(FATAL) << "Out of range edge index.";
+        return Value::kNullOutOfRange;
+      }
+      if (edgeIndex == 0) {
+        Edge edge;
+        edge.src = p.src.vid;
+        edge.dst = p.steps[0].dst.vid;
+        edge.type = p.steps[0].type;
+        edge.ranking = p.steps[0].ranking;
+        return edge;
+      } else {
+        Edge edge;
+        edge.src = p.steps[edgeIndex - 1].dst.vid;
+        edge.dst = p.steps[edgeIndex].dst.vid;
+        edge.type = p.steps[edgeIndex].type;
+        edge.ranking = p.steps[edgeIndex].ranking;
+        return edge;
+      }
+    };
+  }
   {
     auto &attr = functions_["json_extract"];
     // note, we don't support second argument(path) like MySQL JSON_EXTRACT for now

diff --git a/src/graph/util/ParserUtil.cpp b/src/graph/util/ParserUtil.cpp
@@ -59,6 +59,11 @@ void ParserUtil::rewriteLC(QueryContext *qctx,
             node->setAlias(newVarName);
           }
         }
+        for (auto &edge : matchPath.edges()) {
+          if (edge->alias() == oldVarName) {
+            edge->setAlias(newVarName);
+          }
+        }
         return static_cast<Expression *>(mpp);
       } break;
       default:

diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp
@@ -1254,7 +1254,7 @@ Status MatchValidator::validatePathInWhere(
     }
   }
   for (const auto &node : matchPath->nodes()) {
-    if (node->variableDefinedSource() == MatchNode::VariableDefinedSource::kExpression) {
+    if (node->variableDefinedSource() == VariableDefinedSource::kExpression) {
       // Checked in visitor
       continue;
     }
@@ -1273,6 +1273,10 @@ Status MatchValidator::validatePathInWhere(
     }
   }
   for (const auto &edge : matchPath->edges()) {
+    if (edge->variableDefinedSource() == VariableDefinedSource::kExpression) {
+      // Checked in visitor
+      continue;
+    }
     if (!edge->alias().empty()) {
       const auto find = availableAliases.find(edge->alias());
       if (find == availableAliases.end()) {

diff --git a/src/graph/visitor/ValidatePatternExpressionVisitor.cpp b/src/graph/visitor/ValidatePatternExpressionVisitor.cpp
@@ -23,7 +23,7 @@ void ValidatePatternExpressionVisitor::visit(MatchPathPatternExpression *expr) {
   DCHECK(ok()) << expr->toString();
   // don't need to process sub-expression
   const auto &matchPath = expr->matchPath();
-  std::vector<Expression *> nodeFilters;
+  std::vector<Expression *> nodeOrEdgeFilters;
   auto *pathList = InputPropertyExpression::make(pool_, matchPath.toString());
   auto listElementVar = vctx_->anonVarGen()->getVar();
   for (std::size_t i = 0; i < matchPath.nodes().size(); ++i) {
@@ -33,7 +33,7 @@ void ValidatePatternExpressionVisitor::visit(MatchPathPatternExpression *expr) {
       if (find != localVariables_.end()) {
         // TODO we should check variable is Node type
         // from local variable
-        node->setVariableDefinedSource(MatchNode::VariableDefinedSource::kExpression);
+        node->setVariableDefinedSource(VariableDefinedSource::kExpression);
         auto *listElement = VariableExpression::makeInner(pool_, listElementVar);
         // Note: this require build path by node pattern order
         auto *listElementId = FunctionCallExpression::make(
@@ -46,13 +46,37 @@ void ValidatePatternExpressionVisitor::visit(MatchPathPatternExpression *expr) {
         auto *nodeValue = VariableExpression::makeInner(pool_, node->alias());
         auto *nodeId = FunctionCallExpression::make(pool_, "id", {nodeValue});
         auto *equal = RelationalExpression::makeEQ(pool_, listElementId, nodeId);
-        nodeFilters.emplace_back(equal);
+        nodeOrEdgeFilters.emplace_back(equal);
       }
     }
   }
-  if (!nodeFilters.empty()) {
+  for (std::size_t i = 0; i < matchPath.edges().size(); ++i) {
+    const auto &edge = matchPath.edges()[i];
+    if (!edge->alias().empty()) {
+      const auto find = std::find(localVariables_.begin(), localVariables_.end(), edge->alias());
+      if (find != localVariables_.end()) {
+        // TODO we should check variable is Edge type
+        // from local variable
+        edge->setVariableDefinedSource(VariableDefinedSource::kExpression);
+        auto *listElement = VariableExpression::makeInner(pool_, listElementVar);
+        // Note: this require build path by node pattern order
+        auto *edgeInPath = FunctionCallExpression::make(
+            pool_,
+            "_edge",
+            {listElement, ConstantExpression::make(pool_, static_cast<int64_t>(i))});
+        // The alias of node is converted to a inner variable.
+        // e.g. MATCH (v:player) WHERE [t in [v] | (v)-[:like]->(t)] RETURN v
+        // More cases could be found in PathExprRefLocalVariable.feature
+        auto *edgeValue = VariableExpression::makeInner(pool_, edge->alias());
+        auto *equal = RelationalExpression::makeEQ(pool_, edgeInPath, edgeValue);
+        nodeOrEdgeFilters.emplace_back(equal);
+      }
+    }
+  }
+
+  if (!nodeOrEdgeFilters.empty()) {
     auto genList = ListComprehensionExpression::make(
-        pool_, listElementVar, pathList, andAll(nodeFilters), nullptr);
+        pool_, listElementVar, pathList, andAll(nodeOrEdgeFilters), nullptr);
     expr->setGenList(genList);
   }
 }

diff --git a/src/parser/MatchPath.h b/src/parser/MatchPath.h
@@ -79,6 +79,12 @@ class MatchEdgeProp final {
   std::unique_ptr<MatchStepRange> range_;
 };
 
+enum class VariableDefinedSource {
+  kUnknown,
+  kExpression,   // from upper expression
+  kMatchClause,  // from previous match clause
+};
+
 class MatchEdge final {
  public:
   using Direction = nebula::storage::cpp2::EdgeDirection;
@@ -99,6 +105,10 @@ class MatchEdge final {
     return direction_;
   }
 
+  void setAlias(const std::string& alias) {
+    alias_ = alias;
+  }
+
   const std::string& alias() const {
     return alias_;
   }
@@ -115,6 +125,14 @@ class MatchEdge final {
     return range_.get();
   }
 
+  VariableDefinedSource variableDefinedSource() const {
+    return variableDefinedSource_;
+  }
+
+  void setVariableDefinedSource(VariableDefinedSource source) {
+    variableDefinedSource_ = source;
+  }
+
   std::string toString() const;
 
   MatchEdge clone() const {
@@ -139,6 +157,8 @@ class MatchEdge final {
   std::vector<std::unique_ptr<std::string>> types_;
   std::unique_ptr<MatchStepRange> range_;
   MapExpression* props_{nullptr};
+  // Only used for pattern expression
+  VariableDefinedSource variableDefinedSource_{VariableDefinedSource::kUnknown};
 };
 
 class MatchNodeLabel final {
@@ -218,9 +238,9 @@ class MatchNodeLabelList final {
 
 class MatchNode final {
  public:
-  MatchNode(const std::string& alias = "",
-            MatchNodeLabelList* labels = nullptr,
-            Expression* props = nullptr) {
+  explicit MatchNode(const std::string& alias = "",
+                     MatchNodeLabelList* labels = nullptr,
+                     Expression* props = nullptr) {
     alias_ = alias;
     labels_.reset(labels);
     props_ = static_cast<MapExpression*>(props);
@@ -260,12 +280,6 @@ class MatchNode final {
     return me;
   }
 
-  enum class VariableDefinedSource {
-    kUnknown,
-    kExpression,   // from upper expression
-    kMatchClause,  // from previous match clause
-  };
-
   VariableDefinedSource variableDefinedSource() const {
     return variableDefinedSource_;
   }

diff --git a/src/storage/query/GetNeighborsProcessor.h b/src/storage/query/GetNeighborsProcessor.h
@@ -23,7 +23,6 @@ extern ProcessorCounters kGetNeighborsCounters;
  */
 class GetNeighborsProcessor
     : public QueryBaseProcessor<cpp2::GetNeighborsRequest, cpp2::GetNeighborsResponse> {
-
  public:
   /**
    * @brief Construct instance of GetNeighborsProcessor

diff --git a/tests/tck/features/match/Base.feature b/tests/tck/features/match/Base.feature
@@ -1291,8 +1291,17 @@ Feature: Basic match
       WHERE size([ii in e WHERE (v)-[ii]-(v2) | ii])>1
       RETURN count(*) AS cnt
       """
-    # FIXME(czp): Fix this case after https://github.com/vesoft-inc/nebula/issues/5289 closed
-    Then a SemanticError should be raised at runtime: PatternExpression are not allowed to introduce new variables: `ii'.
+    Then the result should be, in any order, with relax comparison:
+      | cnt |
+      | 0   |
+    When executing query:
+      """
+      MATCH p=(v:player)-[]->() where [ii in relationships(p) where (v)-[ii]->()]
+      RETURN count(*) AS cnt
+      """
+    Then the result should be, in any order, with relax comparison:
+      | cnt |
+      | 243 |
 
 # Then the result should be, in any order:
 # | cnt |

diff --git a/tests/tck/features/match/PathExprRefLocalVariable.feature b/tests/tck/features/match/PathExprRefLocalVariable.feature
@@ -122,6 +122,27 @@ Feature: Path expression reference local defined variables
       | "Vince Carter"      |
       | "Rajon Rondo"       |
       | "Ray Allen"         |
+    When executing query:
+      """
+      MATCH p=(v:player)-[]-() where [ii in nodes(p) where (v)-[:like]->(ii)] RETURN count(*) AS count
+      """
+    Then the result should be, in any order:
+      | count |
+      | 133   |
+    When executing query:
+      """
+      MATCH p=(v:player)-[]->() where [ii in relationships(p) where (v)-[ii]->(:team)] return count(*) AS count
+      """
+    Then the result should be, in any order:
+      | count |
+      | 152   |
+    When executing query:
+      """
+      MATCH p=(v:player{name:"Tim Duncan"})-[]->() where [ii in relationships(p) where (v)-[ii]->(:team)] return count(*) AS count
+      """
+    Then the result should be, in any order:
+      | count |
+      | 1     |
 
   Scenario: In With
     When executing query:
@@ -194,6 +215,26 @@ Feature: Path expression reference local defined variables
       | p                                                                                                                                                                                               |
       | [[<("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:serve@0 {end_year: 2016, start_year: 1997}]->("Spurs" :team{name: "Spurs"})>]] |
       | [[<("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:serve@0 {end_year: 2016, start_year: 1997}]->("Spurs" :team{name: "Spurs"})>]] |
+    When executing query:
+      """
+      MATCH (v:player)-[e:like{likeness:80}]->() WITH [ii in [e] | (v)-[ii]->()] AS p return p
+      """
+    Then the result should be, in any order, with relax comparison:
+      | p                                                                                                                                                                                                           |
+      | [[<("Luka Doncic" :player{age: 20, name: "Luka Doncic"})-[:like@0 {likeness: 80}]->("James Harden" :player{age: 29, name: "James Harden"})>]]                                                               |
+      | [[<("Joel Embiid" :player{age: 25, name: "Joel Embiid"})-[:like@0 {likeness: 80}]->("Ben Simmons" :player{age: 22, name: "Ben Simmons"})>]]                                                                 |
+      | [[<("Jason Kidd" :player{age: 45, name: "Jason Kidd"})-[:like@0 {likeness: 80}]->("Vince Carter" :player{age: 42, name: "Vince Carter"})>]]                                                                 |
+      | [[<("James Harden" :player{age: 29, name: "James Harden"})-[:like@0 {likeness: 80}]->("Russell Westbrook" :player{age: 30, name: "Russell Westbrook"})>]]                                                   |
+      | [[<("Dirk Nowitzki" :player{age: 40, name: "Dirk Nowitzki"})-[:like@0 {likeness: 80}]->("Jason Kidd" :player{age: 45, name: "Jason Kidd"})>]]                                                               |
+      | [[<("Dirk Nowitzki" :player{age: 40, name: "Dirk Nowitzki"})-[:like@0 {likeness: 80}]->("Steve Nash" :player{age: 45, name: "Steve Nash"})>]]                                                               |
+      | [[<("Danny Green" :player{age: 31, name: "Danny Green"})-[:like@0 {likeness: 80}]->("LeBron James" :player{age: 34, name: "LeBron James"})>]]                                                               |
+      | [[<("Damian Lillard" :player{age: 28, name: "Damian Lillard"})-[:like@0 {likeness: 80}]->("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"})>]]                                               |
+      | [[<("Tiago Splitter" :player{age: 34, name: "Tiago Splitter"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>]]     |
+      | [[<("Shaquille O'Neal" :player{age: 47, name: "Shaquille O'Neal"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>]] |
+      | [[<("Boris Diaw" :player{age: 36, name: "Boris Diaw"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>]]             |
+      | [[<("Boris Diaw" :player{age: 36, name: "Boris Diaw"})-[:like@0 {likeness: 80}]->("Tony Parker" :player{age: 36, name: "Tony Parker"})>]]                                                                   |
+      | [[<("Ben Simmons" :player{age: 22, name: "Ben Simmons"})-[:like@0 {likeness: 80}]->("Joel Embiid" :player{age: 25, name: "Joel Embiid"})>]]                                                                 |
+      | [[<("Aron Baynes" :player{age: 32, name: "Aron Baynes"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>]]           |
 
   Scenario: In Return
     When executing query:
@@ -224,6 +265,14 @@ Feature: Path expression reference local defined variables
       | [<("Luka Doncic" :player{age: 20, name: "Luka Doncic"})-[:like@0 {likeness: 80}]->("James Harden" :player{age: 29, name: "James Harden"})>]                                                               | [] | "Luka Doncic"      |
       | [<("Shaquille O'Neal" :player{age: 47, name: "Shaquille O'Neal"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>] | [] | "Shaquille O'Neal" |
       | [<("Tiago Splitter" :player{age: 34, name: "Tiago Splitter"})-[:like@0 {likeness: 80}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>]     | [] | "Tiago Splitter"   |
+    When executing query:
+      """
+      MATCH p1 = (v:player{name: 'Tim Duncan'})-[:like]->() return [t in relationships(p1) | (v)-[t]->()] AS p2
+      """
+    Then the result should be, in any order:
+      | p2                                                                                                                                                                                                    |
+      | [[<("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})-[:like@0 {likeness: 95}]->("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"})>]] |
+      | [[<("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})-[:like@0 {likeness: 95}]->("Tony Parker" :player{age: 36, name: "Tony Parker"})>]]     |
 
   Scenario: In Unwind
     When executing query:
@@ -234,6 +283,15 @@ Feature: Path expression reference local defined variables
       | p                                                                                                                                                                                             |
       | [<("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:serve@0 {end_year: 2016, start_year: 1997}]->("Spurs" :team{name: "Spurs"})>] |
       | [<("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:serve@0 {end_year: 2016, start_year: 1997}]->("Spurs" :team{name: "Spurs"})>] |
+    When executing query:
+      """
+      MATCH p1 = (v:player{name: 'Tony Parker'})-[:like]->(), (t:team {name: "Spurs"}) UNWIND [t in relationships(p1) | (v)-[t]->()] AS p2 RETURN p2
+      """
+    Then the result should be, in any order:
+      | p2                                                                                                                                                                                              |
+      | [<("Tony Parker" :player{age: 36, name: "Tony Parker"})-[:like@0 {likeness: 90}]->("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"})>]                                           |
+      | [<("Tony Parker" :player{age: 36, name: "Tony Parker"})-[:like@0 {likeness: 95}]->("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"})>]                                                   |
+      | [<("Tony Parker" :player{age: 36, name: "Tony Parker"})-[:like@0 {likeness: 95}]->("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"})>] |
     When executing query:
       """
       MATCH (v:player{name: 'Tim Duncan'})-[e:like*1..3]->(n), (t:team {name: "Spurs"}) WITH v, e, collect(distinct n) AS ns UNWIND [n in ns | ()-[e*2..4]->(n:player)] AS p RETURN count(p) AS count
-Original file line number
+Diff line change
@@ Expand Up / @@ -23,7 +23,6 @@ extern ProcessorCounters kGetNeighborsCounters; @@
      */
     class GetNeighborsProcessor
         : public QueryBaseProcessor<cpp2::GetNeighborsRequest, cpp2::GetNeighborsResponse> {
      public:
       /**
        * @brief Construct instance of GetNeighborsProcessor
@@ Expand Down @@