-
Notifications
You must be signed in to change notification settings - Fork 46
fix: Fix pushdown conjuncts instead of generating join columns #627
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
base: main
Are you sure you want to change the base?
fix: Fix pushdown conjuncts instead of generating join columns #627
Conversation
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.
Pull Request Overview
This PR refactors the join optimization strategy by removing the "join columns" approach in favor of pushing down filter conjuncts (predicates). The change eliminates unnecessary projections and simplifies the join implementation.
Key Changes:
- Removed join column generation logic and replaced it with direct conjunct pushdown
- Refactored
extractNonInnerJoinEqualitiesto return the left table and handle both single-table and multi-table cases - Simplified join edge logic by removing leftColumns/rightColumns vectors and their associated expressions
- Modified join placement to handle filter pushdown based on join optionality
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| axiom/optimizer/tests/JoinTest.cpp | Updated test expectations to reflect new behavior where right joins convert to left joins and projections are removed |
| axiom/optimizer/tests/DerivedTablePrinterTest.cpp | Fixed aggregate expression to use correct table reference (t3.y instead of dt1.y) |
| axiom/optimizer/tests/CMakeLists.txt | Removed JoinTest.cpp from build configuration |
| axiom/optimizer/ToGraph.h | Renamed translateJoin to addJoin and removed addJoinColumns helper method |
| axiom/optimizer/ToGraph.cpp | Refactored join translation logic to use conjunct pushdown instead of join columns |
| axiom/optimizer/QueryGraph.h | Removed join column vectors and simplified join edge logic with updated isInner/isNonCommutative checks |
| axiom/optimizer/QueryGraph.cpp | Fixed typo ('outr' → 'outer'), cleaned up template usage, and updated function signatures |
| axiom/optimizer/PlanObject.h | Added null pointer checks to PlanObjectSet methods |
| axiom/optimizer/Plan.cpp | Removed join column expressions from downstream column tracking |
| axiom/optimizer/Optimization.h | Renamed placeConjuncts parameter from allowNondeterministic to joinsPlaced |
| axiom/optimizer/Optimization.cpp | Removed ProjectionBuilder and join column mapping logic, simplified join construction |
| axiom/optimizer/DerivedTable.cpp | Added logic to prevent pushdown to optional sides of joins during conjunct distribution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
75534a6 to
11aa549
Compare
axiom/optimizer/tests/CMakeLists.txt
Outdated
| FeatureGen.cpp | ||
| Genies.cpp | ||
| TestConnectorQueryTest.cpp | ||
| JoinTest.cpp |
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.
it is written twice in the current main
122373d to
4e2fcb5
Compare
ad336ee to
5b51a78
Compare
5b51a78 to
eefd583
Compare
Adjust how do we pushdown and place conjuncts (filter expressions/predicates) instead of generate join columns.
"join columns" is design decision with different flaws: