-
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
Fix data flow checking. #4025
Fix data flow checking. #4025
Conversation
I want to know why to erase the |
I'm not sure, it's code in |
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.
I don't remember why to add this check here. Maybe we should find the root cause later. At present, LGTM.
As I known, it's aimed to keep input/output relationship is subset of dependencies relationship. |
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Description:
Thie happens when I try to push filter down to
AppendVertices
, the ldbc query(as below) report broken promise error when I add this rule. After debugging, I found it's caused by mistake ofcheckDataFlow
:AppendVertices
read byArgument
operator andArgument
doesn't depend on it.checkDataFlow
should return failure, but don't for the empty dependencies ofArgument
operator.readby
relationship is erased, and newreadby
relationship is rebuild bydependencies
relationship. But as said before,Argument
operator don't depends onAppendVertices
, so nowArgument
don't readAppendVertices
too!Argument
byread by
relationship, but nowArgument
lost these, so lead to broken promise error.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: