From 8fff4aa91cc29e4edb60b2d91ac236f71e919ec2 Mon Sep 17 00:00:00 2001 From: Yichen Wang <18348405+Aiee@users.noreply.github.com> Date: Fri, 2 Dec 2022 14:48:28 +0800 Subject: [PATCH] Fix dup alias in MATCH (#4940) * Fix dup alias in MATCH * Fix UT * Fix edge case --- src/graph/planner/match/MatchPathPlanner.cpp | 47 ++++++++++++++----- .../validator/test/MatchValidatorTest.cpp | 12 +++++ .../features/bugfix/DupAliasInMatch.feature | 45 ++++++++++++++++++ 3 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 tests/tck/features/bugfix/DupAliasInMatch.feature diff --git a/src/graph/planner/match/MatchPathPlanner.cpp b/src/graph/planner/match/MatchPathPlanner.cpp index e7935889a58..3c534cb6eb4 100644 --- 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 index e81157f7c00..694fc4701d4 100644 --- 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 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 new file mode 100644 index 00000000000..ac5b4ed19b2 --- /dev/null +++ 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 |