Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: define sideband optimization hints #705
feat: define sideband optimization hints #705
Changes from 12 commits
4403448
83a1bc5
14b8669
4640fad
ae7b830
a75ba93
d5bad9b
4bfe1af
0ad927e
e290d32
35c7752
4d957a7
90b885d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 struggle with the use of this. Anchor seems sufficient to have referencing between locations and if we're trying to identify engine meaningful, I think better to just allow a hint any value on the computation.
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.
This defines a sharing group. Because there is a potentially one to many relationship for every computation we need some way of defining which group a particular reference is from.
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 get it.
I think you're saying that a single rel could save, let's say, two hashtables. And five operators could consume all/some/none of those two hash tables. I think anchor is more than enough for that. Why do we also need number?
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.
How would you know if the first hashtable or the second hashtable was supposed to be output for that computation group? The same goes for the consuming side -- how do you know if the that computation is destined for the first or the second hashtable slot? Anchor (or computation group id) tells you which relations are consuming it and not where in the relation it is used.
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.
My main thought is the "how the number maps to an internal structure" is opaque to consumers, whether it is one number or two. Whether we have anchor1 and anchor2 as the left and right or anchor1,number1 and anchor1,number2 doesn't impact that the mapping is hidden/implicit.
For now I suggest that we just eliminate the second number.