Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dup alias in MATCH #4940

Merged
merged 4 commits into from
Dec 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 35 additions & 12 deletions src/graph/planner/match/MatchPathPlanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
12 changes: 12 additions & 0 deletions src/graph/validator/test/MatchValidatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions tests/tck/features/bugfix/DupAliasInMatch.feature
Original file line number Diff line number Diff line change
@@ -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 |