-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Enhancement/optimize edge all predicate #5481
Enhancement/optimize edge all predicate #5481
Conversation
4c5d5b1
to
6c86d3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
plz solve the conflicts of this PR |
Ok. |
f4133f8
to
9916f04
Compare
bfc7a1c
to
fe5fb95
Compare
[Optimizer]Embed edge all predicate into Traverse Move rules fix expr util small change fix compile rename rule Split opt rule Push edge all filter fmt smll fix Add tck fix tck fmt Add tck Add tck Add tck tmp fix fix Fix tck
d1848d5
to
08002ff
Compare
@@ -62,8 +62,12 @@ Status TraverseExecutor::buildRequestVids() { | |||
auto vidType = SchemaUtil::propTypeToValueType(metaVidType.get_type()); | |||
for (; iter->valid(); iter->next()) { | |||
const auto& vid = src->eval(ctx(iter)); | |||
DCHECK_EQ(vid.type(), vidType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why delete this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should check why vid type is unexpected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job!
maybe you should test some scenarios which could not push the all
predicates down, such as:
all( i in e where i.likeness>95 AND true);
all(i in e WHERE i.likeness>95 AND v.player.name =="");
all(i in e WHERE i.likeness>95 OR i.likeness < 50);
...
canBePushed_ = false; | ||
return; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you extract these checks into following functions, such as:
bool checkEdgeSchema(const std::string &sym) const;
bool checkTagSchema(const std::string &sym) const;
if so, you could be able to use these in following way:
if (!checkTagSchema(expr->sym())) return;
if (!checkEdgeSchema(expr->sym()) return;
| 15 | Filter | 11 | | {"condition": "((n.player.age>0)==true)"} | | ||
| 11 | AppendVertices | 14 | | | | ||
| 14 | Filter | 13 | | {"condition": "(size($e)>0)"} | | ||
| 13 | Traverse | 1 | | {"edge filter": "(*.likeness>90)"} | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the predicate is sometimes in edge filter
and sometimes in filter
fields of Traverse
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 step filter cannot push to storage layer.
| 53 | Filter | 52 | | {"condition": "(id($friend)!=\"Tony Parker\")"} | | ||
| 52 | AppendVertices | 59 | | | | ||
| 59 | Filter | 60 | | {"condition": "(id($person)==\"Tony Parker\")"} | | ||
| 60 | Traverse | 2 | | {"filter": "((like.likeness>0) AND (like.likeness>1))"} | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could merge the condition into like.likeness>1
later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. Expression normalization is not optimized yet.
ACK. |
* refactor traverse output (#5464) * refactor traverse output * fix pruneproperties error & none_direct_dst * fix test error * fix shortest path * Change the compaction filter logic to let periodic compaction go through custom compaction filter, to gc expired data (#5447) * Push filter down cross join (#5473) * fix comment * push down filter through cross join --------- Co-authored-by: Sophie <[email protected]> * Fix shortest path crash (#5472) * fix crash of geo (#5475) * fix crash of geo * change log(fatal) to log(error) * fix miss arg $GITHUB_OUTPUT (#5478) * Split optimizer rules (#5470) Fix compile small rename Fix tck Fix tck fmt Fix tck Fix tck * Enhancement/optimize edge all predicate (#5481) * fix eval contains filter on storaged (#5485) * fix eval contains filter on storaged * add tck case * add tck case * fix tck * fix lint * fix lint * Fix expression util function (#5487) fmt Co-authored-by: Sophie <[email protected]> * fix ContainsFilter random fail (#5489) * Fixed graphd startup issue (#5493) * fix prunproperties (#5494) * stop the pushing down of not expressions that are not rewritten to proper forms. (#5502) * Fix edge all predicate with rank function (#5503) Co-authored-by: Sophie <[email protected]> * rewrite param in subgraph & path (#5500) * check param in subgraph * rewrite param in path --------- Co-authored-by: Sophie <[email protected]> * Fix concurrent bug about session count (#5496) * Fix regex expression (#5507) * Update requirements.txt (#5512) Solidified tomli version to solve centos7 compatibility issues * Update cluster id (#5514) --------- Co-authored-by: jimingquan <[email protected]> Co-authored-by: Ryan <[email protected]> Co-authored-by: Yee <[email protected]> Co-authored-by: jie.wang <[email protected]> Co-authored-by: George <[email protected]> Co-authored-by: kyle.cao <[email protected]> Co-authored-by: codesigner <[email protected]> Co-authored-by: dutor <[email protected]> Co-authored-by: Cheng Xuntao <[email protected]> Co-authored-by: Yichen Wang <[email protected]>
What type of PR is this?
What problem(s) does this PR solve?
Push edge all predicate down to the storage layer to optimize performance.
Checklist:
Tests:
Affects: