Fix dup alias in MATCH (#4940)

* Fix dup alias in MATCH * Fix UT * Fix edge case
vesoft-inc · Dec 2, 2022 · 8fff4aa · 8fff4aa
1 parent 4731ed9
commit 8fff4aa
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 12 deletions.
diff --git a/src/graph/planner/match/MatchPathPlanner.cpp b/src/graph/planner/match/MatchPathPlanner.cpp
@@ -254,10 +254,12 @@ Status MatchPathPlanner::leftExpandFromNode(
   for (size_t i = startIndex; i > 0; --i) {
     auto& node = nodeInfos[i];
     auto& dst = nodeInfos[i - 1];
-    bool expandInto = nodeAliasesSeenInPattern.find(dst.alias) != nodeAliasesSeenInPattern.end();
+
     if (!node.anonymous) {
       nodeAliasesSeenInPattern.emplace(node.alias);
     }
+    bool expandInto = nodeAliasesSeenInPattern.find(dst.alias) != nodeAliasesSeenInPattern.end();
+
     auto& edge = edgeInfos[i - 1];
     auto traverse = Traverse::make(qctx, subplan.root, spaceId);
     traverse->setSrc(nextTraverseStart);
@@ -292,19 +294,26 @@ Status MatchPathPlanner::leftExpandFromNode(
     }
   }
 
-  auto& node = nodeInfos.front();
-  if (!node.anonymous) {
-    nodeAliasesSeenInPattern.emplace(node.alias);
+  auto& lastNode = nodeInfos.front();
+
+  bool duppedLastAlias =
+      nodeAliasesSeenInPattern.find(lastNode.alias) != nodeAliasesSeenInPattern.end() &&
+      nodeAliasesSeenInPattern.size() > 1;
+  // If the the last alias has been presented in the pattern, we could emit the AppendVertices node
+  // because the same alias always presents in the same entity.
+  if (duppedLastAlias) {
+    return Status::OK();
   }
+
   auto appendV = AppendVertices::make(qctx, subplan.root, spaceId);
   auto vertexProps = SchemaUtil::getAllVertexProp(qctx, spaceId, true);
   NG_RETURN_IF_ERROR(vertexProps);
   appendV->setVertexProps(std::move(vertexProps).value());
   appendV->setSrc(nextTraverseStart);
-  appendV->setVertexFilter(genVertexFilter(node));
+  appendV->setVertexFilter(genVertexFilter(lastNode));
   appendV->setDedup();
   appendV->setTrackPrevPath(!edgeInfos.empty());
-  appendV->setColNames(genAppendVColNames(subplan.root->colNames(), node, !edgeInfos.empty()));
+  appendV->setColNames(genAppendVColNames(subplan.root->colNames(), lastNode, !edgeInfos.empty()));
   subplan.root = appendV;
 
   return Status::OK();
@@ -324,10 +333,12 @@ Status MatchPathPlanner::rightExpandFromNode(
   for (size_t i = startIndex; i < edgeInfos.size(); ++i) {
     auto& node = nodeInfos[i];
     auto& dst = nodeInfos[i + 1];
-    bool expandInto = nodeAliasesSeenInPattern.find(dst.alias) != nodeAliasesSeenInPattern.end();
+
     if (!node.anonymous) {
       nodeAliasesSeenInPattern.emplace(node.alias);
     }
+    bool expandInto = nodeAliasesSeenInPattern.find(dst.alias) != nodeAliasesSeenInPattern.end();
+
     auto& edge = edgeInfos[i];
     auto traverse = Traverse::make(qctx, subplan.root, spaceId);
     traverse->setSrc(nextTraverseStart);
@@ -355,19 +366,31 @@ Status MatchPathPlanner::rightExpandFromNode(
     }
   }
 
-  auto& node = nodeInfos.back();
-  if (!node.anonymous) {
-    nodeAliasesSeenInPattern.emplace(node.alias);
+  auto& lastNode = nodeInfos.back();
+
+  bool duppedLastAlias =
+      nodeAliasesSeenInPattern.find(lastNode.alias) != nodeAliasesSeenInPattern.end() &&
+      nodeAliasesSeenInPattern.size() > 1;
+
+  if (!lastNode.anonymous) {
+    nodeAliasesSeenInPattern.emplace(lastNode.alias);
   }
+
+  // If the the last alias has been presented in the pattern, we could emit the AppendVertices node
+  // because the same alias always presents in the same entity.
+  if (duppedLastAlias) {
+    return Status::OK();
+  }
+
   auto appendV = AppendVertices::make(qctx, subplan.root, spaceId);
   auto vertexProps = SchemaUtil::getAllVertexProp(qctx, spaceId, true);
   NG_RETURN_IF_ERROR(vertexProps);
   appendV->setVertexProps(std::move(vertexProps).value());
   appendV->setSrc(nextTraverseStart);
-  appendV->setVertexFilter(genVertexFilter(node));
+  appendV->setVertexFilter(genVertexFilter(lastNode));
   appendV->setDedup();
   appendV->setTrackPrevPath(!edgeInfos.empty());
-  appendV->setColNames(genAppendVColNames(subplan.root->colNames(), node, !edgeInfos.empty()));
+  appendV->setColNames(genAppendVColNames(subplan.root->colNames(), lastNode, !edgeInfos.empty()));
   subplan.root = appendV;
 
   return Status::OK();

diff --git a/src/graph/validator/test/MatchValidatorTest.cpp b/src/graph/validator/test/MatchValidatorTest.cpp
@@ -535,6 +535,18 @@ TEST_F(MatchValidatorTest, validateAlias) {
         std::string(result.message()),
         "SemanticError: keywords: vertex and edge are not supported in return clause `EDGE AS b'");
   }
+  // Duplicate alias at the end of the pattern
+  {
+    std::string query = "MATCH (v :person{name:\"Tim Duncan\"})-[e]->(v2)-[]->(v2) RETURN e";
+    std::vector<PlanNode::Kind> expected = {PlanNode::Kind::kProject,
+                                            PlanNode::Kind::kProject,
+                                            PlanNode::Kind::kFilter,
+                                            PlanNode::Kind::kTraverse,
+                                            PlanNode::Kind::kTraverse,
+                                            PlanNode::Kind::kIndexScan,
+                                            PlanNode::Kind::kStart};
+    EXPECT_TRUE(checkResult(query, expected));
+  }
 }
 
 }  // namespace graph

diff --git a/tests/tck/features/bugfix/DupAliasInMatch.feature b/tests/tck/features/bugfix/DupAliasInMatch.feature
@@ -0,0 +1,45 @@
+# Copyright (c) 2022 vesoft inc. All rights reserved.
+#
+# This source code is licensed under Apache 2.0 License.
+# Issue link: https://github.com/vesoft-inc/nebula-ent/issues/1605
+Feature: Duplicate alias in MATCH
+
+  Background:
+    Given a graph with space named "nba"
+
+  Scenario: Duplicate node alias
+    # expand from left to right
+    When executing query:
+      """
+      MATCH (n0)-[]->(n1)-[]->(n1) WHERE (id(n0) == "Tim Duncan") RETURN n1
+      """
+    Then the result should be, in any order:
+      | n1 |
+    # expand from right to left
+    When executing query:
+      """
+      MATCH (n1)<-[]-(n1)<-[]-(n0) WHERE (id(n0) == "Tim Duncan") RETURN n1
+      """
+    Then the result should be, in any order:
+      | n1 |
+    When executing query:
+      """
+      MATCH (n0)-[]->(n1)-[]->(n1)-[]->(n1) WHERE (id(n0) == "Tim Duncan") RETURN n1
+      """
+    Then the result should be, in any order:
+      | n1 |
+    When executing query:
+      """
+      MATCH (n0)-[]->(n1)-[]->(n1)-[]->(n1)-[]->(n1) WHERE (id(n0) == "Tim Duncan") RETURN n1
+      """
+    Then the result should be, in any order:
+      | n1 |
+
+  Scenario: Duplicate node alias expand both direction
+    # expand from middle to both sides
+    When executing query:
+      """
+      MATCH (n1)-[]->(n0)-[]->(n1)-[]->(n1)-[]->(n1) WHERE (id(n0) == "Tim Duncan") RETURN n1
+      """
+    Then the result should be, in any order:
+      | n1 |