-
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: support hint for IndexHashJoin and IndexMergeJoin #13238
Conversation
00bc548
to
efa1b90
Compare
Codecov Report
@@ Coverage Diff @@
## master #13238 +/- ##
===========================================
Coverage 80.5742% 80.5742%
===========================================
Files 471 471
Lines 115419 115419
===========================================
Hits 92998 92998
Misses 15352 15352
Partials 7069 7069 |
b458d33
to
2ae0b6a
Compare
/run-all-tests |
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
if rightJoins != nil && (rightOuter && !leftOuter || rhsCardinality < lhsCardinality) { | ||
return rightJoins, rightOuter | ||
switch { | ||
case p.JoinType == InnerJoin && p.Children()[0].statsInfo().Count() < p.Children()[1].statsInfo().Count(): |
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.
Well, I don't think it's always a better choice for the situation to force left outers. What about remove the second condition?
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.
Then we can get both sides inner join.
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'll affect many plans, using an individual PR is better. This issue traces 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.
LGTM, please revert the change of go.sum
. I'll approve the PR.
30af04b
to
8a800df
Compare
/run-all-tests |
@XuHuaiyu merge failed. |
What problem does this PR solve?
1. fix issue #123092. support hint for IndexHashJoin and IndexMergeJoin
What is changed and how it works?
Check List
Tests
Code changes
N/A
Side effects
N/A
Related changes
N/A
Release note
N/A