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.

Sign up for GitHub

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

Jump to bottom

add tagFilter for traverse #5137

Merged
merged 3 commits into from
Dec 29, 2022
Merged

add tagFilter for traverse #5137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d49a26
add tagFilter for traverse
nevermore3 Dec 28, 2022
6343715
add test case
nevermore3 Dec 28, 2022
c169f69
Merge branch 'master' into fix_issue1668
Sophie-Xie Dec 28, 2022
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
2 changes: 1 addition & 1 deletion src/graph/executor/query/TraverseExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ folly::Future<Status> TraverseExecutor::getNeighbors() {
finalStep ? traverse_->orderBy() : std::vector<storage::cpp2::OrderBy>(),
finalStep ? traverse_->limit(qctx()) : -1,
selectFilter(),
nullptr)
currentStep_ == 1 ? traverse_->tagFilter() : nullptr)
.via(runner())
.thenValue([this, getNbrTime](StorageRpcResponse<GetNeighborsResponse>&& resp) mutable {
vids_.clear();
Expand Down
2 changes: 2 additions & 0 deletions src/graph/planner/match/MatchPathPlanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ Status MatchPathPlanner::leftExpandFromNode(size_t startIndex, SubPlan& subplan)
traverse->setVertexProps(std::move(vertexProps).value());
traverse->setEdgeProps(SchemaUtil::getEdgeProps(edge, reversely, qctx, spaceId));
traverse->setVertexFilter(genVertexFilter(node));
traverse->setTagFilter(genVertexFilter(node));
traverse->setEdgeFilter(genEdgeFilter(edge));
traverse->setEdgeDirection(edge.direction);
traverse->setStepRange(edge.range);
Expand Down Expand Up @@ -275,6 +276,7 @@ Status MatchPathPlanner::rightExpandFromNode(size_t startIndex, SubPlan& subplan
traverse->setVertexProps(std::move(vertexProps).value());
traverse->setEdgeProps(SchemaUtil::getEdgeProps(edge, reversely, qctx, spaceId));
traverse->setVertexFilter(genVertexFilter(node));
traverse->setTagFilter(genVertexFilter(node));
traverse->setEdgeFilter(genEdgeFilter(edge));
traverse->setEdgeDirection(edge.direction);
traverse->setStepRange(edge.range);
Expand Down
4 changes: 4 additions & 0 deletions src/graph/planner/plan/Query.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,9 @@ void Traverse::cloneMembers(const Traverse& g) {
if (g.firstStepFilter_ != nullptr) {
setFirstStepFilter(g.firstStepFilter_->clone());
}
if (g.tagFilter_ != nullptr) {
setTagFilter(g.tagFilter_->clone());
}
}

std::unique_ptr<PlanNodeDescription> Traverse::explain() const {
Expand All @@ -794,6 +797,7 @@ std::unique_ptr<PlanNodeDescription> Traverse::explain() const {
addDescription("first step filter",
firstStepFilter_ != nullptr ? firstStepFilter_->toString() : "",
desc.get());
addDescription("tag filter", tagFilter_ != nullptr ? tagFilter_->toString() : "", desc.get());
return desc;
}

Expand Down
9 changes: 9 additions & 0 deletions src/graph/planner/plan/Query.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,14 @@ class Traverse final : public GetNeighbors {
firstStepFilter_ = filter;
}

Expression* tagFilter() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference from vertex filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same, but the vertexfilter will not be pushed down to the storage layer, tagFilter will

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And vertex filter will be moved to firstStepFilter in the optimizer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too much filter is really confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vertexFilter and edgeFilter are in the graph layer. Under some conditions, they cannot be pushed to the storage layer, so they continue to filter at the graph layer. firstStepFilter is the first step version of the filter, which is pushed down to the storage layer, but they cannot filter vertex. So tagFilter is added, if the vertex's attribute does not satisfy tagFilter, it will be filtered out directly in the storage layer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really should not design such a large-grained plan node in the future.

return tagFilter_;
}

void setTagFilter(Expression* tagFilter) {
tagFilter_ = tagFilter;
}

private:
friend ObjectPool;
Traverse(QueryContext* qctx, PlanNode* input, GraphSpaceID space)
Expand All @@ -1657,6 +1665,7 @@ class Traverse final : public GetNeighbors {
bool trackPrevPath_{true};
// Push down filter in first step
Expression* firstStepFilter_{nullptr};
Expression* tagFilter_{nullptr};
};

// Append vertices to a path.
Expand Down
40 changes: 40 additions & 0 deletions tests/tck/features/match/Base.IntVid.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,3 +593,43 @@ Feature: Basic match
Then the result should be, in any order:
| v |
| ("Boris Diaw" :player{age: 36, name: "Boris Diaw"}) |

Scenario: match with tag filter
When executing query:
"""
MATCH (a:team)-[e*0..1]-(b) where id(a) == hash('Tim Duncan') return b
"""
Then the result should be, in any order, with relax comparison:
| b |
When executing query:
"""
MATCH (a:team)-[e*0..0]-(b) where id(a) in [hash('Tim Duncan'), hash('Spurs')] return b
"""
Then the result should be, in any order, with relax comparison:
| b |
| ('Spurs') |
When executing query:
"""
MATCH (a:team)-[e*0..1]-(b) where id(a) in [hash('Tim Duncan'), hash('Spurs')] return b
"""
Then the result should be, in any order, with relax comparison:
| b |
| ("Spurs") |
| ("Aron Baynes") |
| ("Boris Diaw") |
| ("Cory Joseph") |
| ("Danny Green") |
| ("David West") |
| ("Dejounte Murray") |
| ("Jonathon Simmons") |
| ("Kyle Anderson") |
| ("LaMarcus Aldridge") |
| ("Manu Ginobili") |
| ("Marco Belinelli") |
| ("Paul Gasol") |
| ("Rudy Gay") |
| ("Tiago Splitter") |
| ("Tim Duncan") |
| ("Tony Parker") |
| ("Tracy McGrady") |
| ("Marco Belinelli") |
40 changes: 40 additions & 0 deletions tests/tck/features/match/Base.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -970,3 +970,43 @@ Feature: Basic match
MATCH (v{name: "Tim Duncan"}) return v
"""
Then a SemanticError should be raised at runtime: `name:"Tim Duncan"': No tag found for property.

Scenario: match with tag filter
When executing query:
"""
MATCH (a:team)-[e*0..1]-(b) where id(a) == 'Tim Duncan' return b
"""
Then the result should be, in any order, with relax comparison:
| b |
When executing query:
"""
MATCH (a:team)-[e*0..0]-(b) where id(a) in ['Tim Duncan', 'Spurs'] return b
"""
Then the result should be, in any order, with relax comparison:
| b |
| ('Spurs') |
When executing query:
"""
MATCH (a:team)-[e*0..1]-(b) where id(a) in ['Tim Duncan', 'Spurs'] return b
"""
Then the result should be, in any order, with relax comparison:
| b |
| ("Spurs") |
| ("Aron Baynes") |
| ("Boris Diaw") |
| ("Cory Joseph") |
| ("Danny Green") |
| ("David West") |
| ("Dejounte Murray") |
| ("Jonathon Simmons") |
| ("Kyle Anderson") |
| ("LaMarcus Aldridge") |
| ("Manu Ginobili") |
| ("Marco Belinelli") |
| ("Paul Gasol") |
| ("Rudy Gay") |
| ("Tiago Splitter") |
| ("Tim Duncan") |
| ("Tony Parker") |
| ("Tracy McGrady") |
| ("Marco Belinelli") |