-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
planner/cascades: support outer join implementation and push down selection for outer join #13996
Conversation
"SQL": "select t2.a, t2.b from t1 right join t2 on t1.a = t2.a where t1.a > 2 and t2.b > 200", | ||
"Plan": [ | ||
"Projection_12 10000.00 root test.t2.a, test.t2.b", | ||
"└─HashRightJoin_14 10000.00 root right outer join, inner:TableReader_17 (REVERSED), equal:[eq(test.t1.a, test.t2.a)]", |
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.
Where is the t1.a>2
?
Codecov Report
@@ Coverage Diff @@
## master #13996 +/- ##
===========================================
Coverage ? 80.2254%
===========================================
Files ? 484
Lines ? 121707
Branches ? 0
===========================================
Hits ? 97640
Misses ? 16322
Partials ? 7745 |
planner/core/logical_plans.go
Outdated
@@ -320,6 +322,9 @@ type LogicalSelection struct { | |||
// but after we converted to CNF(Conjunctive normal form), it can be | |||
// split into a list of AND conditions. | |||
Conditions []expression.Expression | |||
// HasBeenPushed means whether the selection is applied rule `PushSelDown`, | |||
// it's used in cascades planner. | |||
HasBeenPushed bool |
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.
We need a general solution to solve the recursive rule applying. There's also other cases that would cause endless loop.
You can add a TODO here first.
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.
OK.
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 will be done by AppliedRuleSet
. When we apply the rule PushSelDownJoin
, we should add this rule to AppliedRuleSet
of the new created Selection, so that the new Selection will not match this rule again.
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.
Where is the AppliedRuleSet
?
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'm still working on it >.<
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.
OK...Don't forget to remove this when you finish it. 0.0
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.
lgtm
The field |
That's alright. Go ahead. |
/run-unit-test |
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.
LGTM
/run-all-tests |
@lzmhhh123 merge failed. |
/run-integration-copr-test |
/merge |
/run-all-tests |
What problem does this PR solve?
As the title says.
What is changed and how it works?
support outer join implementation rule and push down selection for outer join
Check List
Tests
Code changes