planner/core: raise warning for unmatched join hint #9914
Conversation
alivxxx
added
type/enhancement
planner/core/planbuilder.go
Outdated
| if curEntry.L == tableName.L { | ||
| joinHint.matched = true |
There was a problem hiding this comment.
Do we need to mark hint as unmatched if any specified hint table cannot find a match? /cc @morgo
There was a problem hiding this comment.
The default value is false, is unmatched?
There was a problem hiding this comment.
I mean if the hint contains t1 and t2, while the query only involves t2, should we raise warning?
There was a problem hiding this comment.
If so, maybe it's more reasonable to raise a warning.
There was a problem hiding this comment.
Can we define the matched as a string slice?
Thus we can print the accurate wrong table name in the warning message
There was a problem hiding this comment.
Warning sounds good to me. Note that in some cases, MySQL hints do not even raise warnings because they are considered hints (entirely optional to enforce vs. directives). I think we should just arrive at warnings and never errors.
planner/core/planbuilder.go
Outdated
| if curEntry.L == tableName.L { | ||
| joinHint.matched = true |
There was a problem hiding this comment.
The default value is false, is unmatched?
Codecov Report
@@ Coverage Diff @@
## master #9914 +/- ##
==========================================
Coverage ? 77.471%
==========================================
Files ? 403
Lines ? 81806
Branches ? 0
==========================================
Hits ? 63376
Misses ? 13710
Partials ? 4720 |
eurekaka
added
status/LGT2
|
/run-all-tests |
|
/run-unit-test |
|
@lamxTyler please cherry pick this PR to release-2.1 |
What problem does this PR solve?
When the join hint is incorrect, for example when it needs to use the table alias name while the table name in the hint is the original name, we can raise a warning to the client.
What is changed and how it works?
Add a new field to record whether the join hint is matched, and check it when we pop the table hints, if it is not matched, raise an warning.
Check List
Tests
Code changes
Side effects
Related changes