[DYOD] Only use valid UCCs in optimization and limit cacheability of plans optimized using UCCs #2600
Conversation
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
8016545 to
1ef774f
Compare
SanJSp
reviewed
Aug 4, 2023
There was a problem hiding this comment.
A looooot of changes, but good job!
Again only minor comments and all in all great changes! Feel free to consider my comments and if they're not relevant, skip them.
For the future: Maybe, when touching so many files, it makes sense to specify an order in which the reviewer can work through the files.
Happy coding!
src/lib/logical_query_plan/data_dependencies/functional_dependency.cpp
Outdated
Show resolved
Hide resolved
src/lib/logical_query_plan/data_dependencies/functional_dependency.hpp
Outdated
Show resolved
Hide resolved
src/lib/optimizer/strategy/dependent_group_by_reduction_rule.cpp
Outdated
Show resolved
Hide resolved
src/test/lib/optimizer/strategy/join_to_predicate_rewrite_rule_test.cpp
Outdated
Show resolved
Hide resolved
d214251 to
b3b721c
Compare
…uaranteed to be currently valid Co-authored-by: kalathen <kalathen@jyu.fi> Co-authored-by: Margarete <margarete.dippel@student.hpi.de>
…y be false if they were derived from a table key constraint which can become invalid Co-authored-by: kalathen <kalathen@jyu.fi> Co-authored-by: Margarete <margarete.dippel@student.hpi.de>
…e resulting query plan is cacheable Co-authored-by: kalathen <kalathen@jyu.fi> Co-authored-by: Margarete <margarete.dippel@student.hpi.de>
… we need to be able to call is_permanent on it Co-authored-by: kalathen <kalathen@jyu.fi> Co-authored-by: Margarete <margarete.dippel@student.hpi.de>
…query plan and only cache cacheable query plans Co-authored-by: kalathen <kalathen@jyu.fi> Co-authored-by: Margarete <margarete.dippel@student.hpi.de>
Co-authored-by: kalathen <kalathen@jyu.fi> Co-authored-by: Margarete <margarete.dippel@student.hpi.de>
Co-authored-by: kalathen <kalathen@jyu.fi> Co-authored-by: Margarete <margarete.dippel@student.hpi.de>
Co-authored-by: kalathen <kalathen@jyu.fi> Co-authored-by: Margarete <margarete.dippel@student.hpi.de>
a5382c1 to
be582b7
Compare
dey4ss
reviewed
Sep 28, 2023
src/lib/logical_query_plan/data_dependencies/functional_dependency.hpp
Outdated
Show resolved
Hide resolved
src/lib/logical_query_plan/data_dependencies/functional_dependency.hpp
Outdated
Show resolved
Hide resolved
src/test/lib/optimizer/strategy/between_composition_rule_test.cpp
Outdated
Show resolved
Hide resolved
src/test/lib/optimizer/strategy/expression_reduction_rule_test.cpp
Outdated
Show resolved
Hide resolved
src/test/lib/optimizer/strategy/in_expression_rewrite_rule_test.cpp
Outdated
Show resolved
Hide resolved
dey4ss
reviewed
Sep 28, 2023
|
Put this on hold until end of August. We plan to incorporate this feature, but there is currently no capacity for it. Will work on it when back from the internship. |
a9edd5a to
be582b7
Compare
…erg/only_use_valid_uccs_in_optimization
…erg/only_use_valid_uccs_in_optimization
… `is_time_dependent`
…SQLPipelineStatement to be a reference
… and independent FDs. add tests for correct handling of cacheability of LQPs using FDs for optimization.
…,add integration test of ucc discovery plugin, switch to invalidation instead of deletion for invalidated key constraints
3c808d7 to
b919c53
Compare
b919c53 to
d0614ae
Compare
…erg/only_use_valid_uccs_in_optimization
…les' into j-hellenberg/only_use_valid_uccs_in_optimization
Rob2U
added a commit
that referenced
this pull request
Apr 30, 2025
… depending on it (this functionality is now part #2600)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Make sure to only use UCCs in optimization that are
guaranteed_to_be_valid.Also, we want to ensure that Hyrise only caches logical and physical query plans that cannot become invalid, because executing an invalid query plan might produce incorrect results. Query plans could become invalid if they have been optimized using UCCs that are not permanent, e.g., incidental UCCs discovered by the
UccDiscoveryPlugin. The same can happen if a Functional Dependency (FD) was used that was derived from a non-permanent UCC. We ensure correct caching by having each optimization rule return whether the resulting Logical Query Plan can be cached.Note that this PR currently targets our first PR branch, in order to not include the changes from there again here :) Please review the other PR before this one. We will change the PR to target master before merging.