add vertex filter for GetNeighbors #4671
Conversation
nevermore3
force-pushed
the
fix_getNeighbor_src_filter
branch
3 times, most recently
from
d8c44e9 to
2b7221f
Compare
|
Please add some description of what this pr for, from code it seems pushdown vertex filter to storage, while graph side logic seems not changed, only pass nullptr to the new added vertex_filte. |
Yes, the getNeightbor interface of the graph side behaves the same as before， it is only used for subgraphs |
| @@ -152,6 +152,13 @@ nebula::cpp2::ErrorCode QueryBaseProcessor<REQ, RESP>::buildFilter( | |||
| if (filter_ == nullptr) { | |||
| return nebula::cpp2::ErrorCode::E_INVALID_FILTER; | |||
| } | |||
| const auto* vertexFilterStr = getFilter(req, true); | |||
There was a problem hiding this comment.
Be careful here, if the filter is vertex-only, both filter_ and vertexFilter_ will be set, the Filter will eval more than once?
There was a problem hiding this comment.
Yes, it will be executed twice if vertexFilter_ and filter_ are the same And the result is true
There was a problem hiding this comment.
Is filter and vertex-filter mutually exclusive? (can't set them both at the same request)? If is, we only need to eval once I think~
BTW, where do we extract vertex filter? I didn't see the code to set it.
There was a problem hiding this comment.
vertexFilter_ is a subset of filter_, and this feature is not universal (
because we can't handle logicalOR expressions with mixed vertex and edges. like $^.player.age > 30 OR like.likeness > 100), it is only used for subgraphs, there will be code extracted by vertexfilter in #4357
nevermore3
force-pushed
the
fix_getNeighbor_src_filter
branch
from
7ff60a1 to
2bd9307
Compare
After discussing with Peng Wei, this is the final solution |
src/storage/exec/FilterNode.h
Outdated
| @@ -93,6 +99,13 @@ class FilterNode : public IterateNode<T> { | |||
| // return true when the value iter points to a value which can filter | |||
| bool checkTagAndEdge() { | |||
| expCtx_->reset(this->reader(), this->key().str()); | |||
| if (vertexFilterExp_ != nullptr) { | |||
There was a problem hiding this comment.
should this be also evaluated in checkTagOnly?
There was a problem hiding this comment.
will not be used in this scenario
| @@ -152,6 +152,13 @@ nebula::cpp2::ErrorCode QueryBaseProcessor<REQ, RESP>::buildFilter( | |||
| if (filter_ == nullptr) { | |||
| return nebula::cpp2::ErrorCode::E_INVALID_FILTER; | |||
| } | |||
| const auto* vertexFilterStr = getFilter(req, true); | |||
| if (vertexFilterStr != nullptr && !vertexFilterStr->empty()) { | |||
| vertexFilter_ = Expression::decode(pool, *vertexFilterStr); | |||
There was a problem hiding this comment.
should vertexFilter_ be check in checkExp?
There was a problem hiding this comment.
vertexfilter is a subset of filter_, so does not need to be checked repeatedly
nevermore3
force-pushed
the
fix_getNeighbor_src_filter
branch
from
2bd9307 to
31aec11
Compare
| @@ -152,6 +152,13 @@ nebula::cpp2::ErrorCode QueryBaseProcessor<REQ, RESP>::buildFilter( | |||
| if (filter_ == nullptr) { | |||
| return nebula::cpp2::ErrorCode::E_INVALID_FILTER; | |||
| } | |||
| const auto* vertexFilterStr = getFilter(req, true); | |||
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
close #4649
Description:
Add vertexFilter to getNeighbor interface, when the filter condition of the vertex is not satisfied, do not return the data for this vertex and corresponding edge
How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: