Fix join on kEdgeList (#5124)

add tck add tck add tck Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
vesoft-inc · Dec 28, 2022 · 3c2c9ca · 3c2c9ca
1 parent f519503
commit 3c2c9ca
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 13 deletions.
diff --git a/src/common/datatypes/Edge.cpp b/src/common/datatypes/Edge.cpp
@@ -144,6 +144,7 @@ bool Edge::keyEqual(const Edge& rhs) const {
 
 std::string Edge::id() const {
   std::string s;
+  // FIXME(czp): How to distinguish between `11->2@90` and `1->12@90`?
   if (src.type() == Value::Type::INT) {
     EdgeType t = type > 0 ? type : -type;
     const int64_t& srcId = type > 0 ? src.getInt() : dst.getInt();

diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp
@@ -1868,7 +1868,8 @@ FunctionManager::FunctionManager() {
           return value.getEdge().id();
         }
         default: {
-          return Value::kNullBadType;
+          // Join on the origin type
+          return args[0];
         }
       }
     };

diff --git a/src/graph/planner/match/MatchPlanner.cpp b/src/graph/planner/match/MatchPlanner.cpp
@@ -72,23 +72,18 @@ Status MatchPlanner::connectMatchPlan(SubPlan& queryPlan, MatchClauseContext* ma
   for (auto& alias : matchCtx->aliasesGenerated) {
     auto it = matchCtx->aliasesAvailable.find(alias.first);
     if (it != matchCtx->aliasesAvailable.end()) {
-      // Joined type should be same,
-      // If any type is kRuntime, leave the type check to runtime,
-      // Primitive types (Integer, String, etc.) or composite types(List, Map etc.)
-      // are deduced to kRuntime when cannot be deduced during planning.
-      if (it->second != alias.second && it->second != AliasType::kRuntime &&
-          alias.second != AliasType::kRuntime) {
+      interAliases.insert(alias.first);
+      // If any type is kRuntime, leave the type check to runtime
+      if (it->second == AliasType::kRuntime || alias.second == AliasType::kRuntime) {
+        continue;
+      }
+      // Non-runtime joined types should be the same
+      if (it->second != alias.second) {
         return Status::SemanticError(fmt::format("{} binding to different type: {} vs {}",
                                                  alias.first,
                                                  AliasTypeName::get(alias.second),
                                                  AliasTypeName::get(it->second)));
       }
-      // Joined On EdgeList is not supported
-      if (alias.second == AliasType::kEdgeList) {
-        return Status::SemanticError("`%s' defined with type EdgeList, which cannot be joined on",
-                                     alias.first.c_str());
-      }
-      interAliases.insert(alias.first);
     }
   }
   if (!interAliases.empty()) {

diff --git a/src/graph/planner/match/SegmentsConnector.cpp b/src/graph/planner/match/SegmentsConnector.cpp
@@ -23,6 +23,8 @@ SubPlan SegmentsConnector::innerJoin(QueryContext* qctx,
   for (auto& alias : intersectedAliases) {
     auto* args = ArgumentList::make(pool);
     args->addArgument(InputPropertyExpression::make(pool, alias));
+    // TODO(czp): We should not do that for all data types,
+    // the InputPropertyExpression may be any data type
     auto* expr = FunctionCallExpression::make(pool, "_joinkey", args);
     hashKeys.emplace_back(expr);
     probeKeys.emplace_back(expr->clone());

diff --git a/tests/tck/features/bugfix/MatchJoinOnEdge.feature b/tests/tck/features/bugfix/MatchJoinOnEdge.feature
@@ -30,3 +30,40 @@ Feature: Test match used in pipe
     Then the result should be, in any order:
       | v                                                                                                           | u                                                     |
       | ("Tim Duncan" :player{age: 42, name: "Tim Duncan"} :bachelor{name: "Tim Duncan", speciality: "psychology"}) | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
+
+  Scenario: Multiple Match joined on edge list
+    When executing query:
+      """
+      MATCH p1=(:player{name:"Tim Duncan"})-[e:like*1..2]->(:player{name:"Tim Duncan"})
+      MATCH p2=(:player{name:"Tim Duncan"})-[e:like*1..2]-(:player{name:"Tim Duncan"})
+      RETURN e[0].likeness AS e1, e[1].likeness AS e2
+      """
+    Then the result should be, in any order:
+      | e1 | e2 |
+      | 95 | 90 |
+      | 95 | 95 |
+    When executing query:
+      """
+      MATCH p1=(:player{name:"Tim Duncan"})-[e:like*1..2]->(:player{name:"Tim Duncan"})
+      WITH relationships(p1) AS e
+      MATCH p2=(:player{name:"Tim Duncan"})-[e:like*1..2]-(:player{name:"Tim Duncan"})
+      WITH relationships(p2) AS e
+      RETURN e[0].likeness AS e1, e[1].likeness AS e2
+      """
+    Then the result should be, in any order:
+      | e1 | e2 |
+      | 95 | 90 |
+      | 95 | 95 |
+    # issue #1538
+    When executing query:
+      """
+      MATCH p1=(v1:player)-[e:like*1..]->(v) where id(v1)=='Tim Duncan'
+      MATCH p2=(v2:player)-[e:like*1..]->(v) where id(v2)=='Tim Duncan'
+      WITH id(v1) AS tagc1, id(v2) AS tagc2,
+      sum(reduce(total=1,ratio in relationships(p1)|total * ratio.likeness)) AS ratio1,
+      sum(reduce(total=1,ratio in relationships(p2)|total * ratio.likeness)) AS ratio2
+      RETURN tagc1,tagc2, ratio1, ratio2;
+      """
+    Then the result should be, in any order:
+      | tagc1        | tagc2        | ratio1        | ratio2        |
+      | "Tim Duncan" | "Tim Duncan" | 2111707594715 | 2111707594715 |