planner: support generating leading and hash_join_build hint from physical plan
#55195
Conversation
ti-chi-bot
bot
added
do-not-merge/needs-linked-issue
release-note-none
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #55195 +/- ##
================================================
+ Coverage 72.8938% 74.2622% +1.3683%
================================================
Files 1571 1571
Lines 439731 440116 +385
================================================
+ Hits 320537 326840 +6303
+ Misses 99471 93056 -6415
- Partials 19723 20220 +497
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/retest |
|
/retest |
|
CI pass now |
ti-chi-bot
bot
added
size/XL
ti-chi-bot
bot
added
size/XL
|
/retest |
time-and-fate
changed the title
leading and hash_join_build hint from physical plan
pkg/planner/core/hint_utils.go
Outdated
| func genJoinMethodHintForSinglePhysicalJoin( | ||
| sctx base.PlanContext, | ||
| joinType string, | ||
| parentOffset int, |
There was a problem hiding this comment.
Could we rename parentOffset to parentQBOffset or parentQueryBlockOffset? Offset seems unclear, better to add keyword QB or QueryBlock into it.
| } | ||
| guessQBOffset = false | ||
| var dbName, tableName *model.CIStr | ||
| if qbOffset != parentOffset { |
There was a problem hiding this comment.
This can only happen when the joined node is from a sub-query? Better to add some comments.
There was a problem hiding this comment.
This logic is not changed. I move the previous comments here.
| ht *ast.HintTable, | ||
| ) { | ||
| qbOffset = joinNode.QueryBlockOffset() | ||
| selfOffset := joinNode.QueryBlockOffset() | ||
| qbOffset = selfOffset | ||
| if qbOffset == -1 { |
There was a problem hiding this comment.
When this can be -1? The optimizer forgot to set this?
There was a problem hiding this comment.
Yes. Sometimes join reorder process won't maintain it and set it to -1.
| if qbOffset < 0 || ht == nil { | ||
| qbOffsets = append(qbOffsets, -1) | ||
| hintTbls = append(hintTbls, nil) |
There was a problem hiding this comment.
Should we "continue" after these 2 append operations?
There was a problem hiding this comment.
Good catch. Updated.
ti-chi-bot
bot
added
size/XXL
|
/hold |
ti-chi-bot
bot
added
the
do-not-merge/hold
ti-chi-bot
bot
added
approved
lgtm
and removed
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qw4990, tangenta, winoros The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
What problem does this PR solve?
Issue Number: close #55280
What changed and how does it work?
As the title says.
The overall structure of methods in
hint_utils.go:genJoinMethodHintForSinglePhysicalJoin()genJoinOrderHintFromRootPhysicalJoin(),extractOrderedPhysicalJoinGroup()genHintTblForJoinNodes(),genHintTblForSingleJoinNode(),extractTableAsName()Please see the comments for details since the changes involve many implementation details.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.