-
Notifications
You must be signed in to change notification settings - Fork 387
feat(queryopt): set simplification optimizer #3051
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
Open
jzelinskie
wants to merge
20
commits into
authzed:main
Choose a base branch
from
jzelinskie:absorption-optim
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
ff7999e
chore(queryopt): add explicit priorities to existing optimizers
jzelinskie 1f13802
feat(queryopt): implement absorption-idempotency optimizer mutation
jzelinskie c2d81d3
feat(queryopt): register absorption-idempotency optimizer
jzelinskie 984ef96
chore(CHANGELOG): add absorption optimizer entry
jzelinskie 3c69f91
feat(queryopt): add complement-absorption to union subsumption (A ∪ (…
jzelinskie 4752935
feat(queryopt): add exclusion self-annihilation (A − A = ∅)
jzelinskie 39fbed8
feat(queryopt): add intersection idempotency/absorption (A ∩ A = A, A…
jzelinskie 9eeabbb
docs(queryopt): update absorption optimizer description with all five…
jzelinskie bf308fc
refactor(queryopt): dedup child iteration
jzelinskie d48db40
feat(queryopt): generalize exclusion annihilation to X − A = ∅ when X…
jzelinskie 028f793
fix(NullPropagation): null if either arrow child is null
jzelinskie 985fb54
test(NullPropagation): add comprehensive tests for all node types
jzelinskie 15bd84c
feat(queryopt): add four new set-theoretic simplification rules
jzelinskie df4bc4a
refactor(queryopt): give each absorption rule its own function
jzelinskie b23ba30
rename(queryopt): absorption-idempotency → set-simplification
jzelinskie 96c89b5
refactor(queryopt): use new outline.Equals alias
jzelinskie 782828b
test(queryopt): add coverage for uncovered branches in set-simplifica…
jzelinskie 7a927b1
fix: test w/ new api for registered optimizations
jzelinskie 8074f0b
refactor: improve legibility of set simplification
jzelinskie 40df164
test(queryopt): achieve 100% coverage in set-simplification
jzelinskie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Do we have a means of constructing a list of optimizers at startup time rather than taking the registration approach? This smells like it's going to turn into
z-indexeventually.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 is definitely a place that could use work. I think for now it'd probably help to just have a central list where optimizers are registered, similar how to we register gRPC middleware. If it ever got too confusing doing that, it'd make sense to explicitly add "before" and "after" properties to each optimizer and let the system compute the order based on those (kind like systemd startup ordering)
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.
@jzelinskie I'd say we do the before/after now: its going to rapidly become untenable, so I recommend as a followup PR