-
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: refactor plan cache LRU code #41618
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
Skipping CI for Draft Pull Request. |
/retest |
util/util.go
Outdated
func (s FieldSlice) CheckTypesCompatibility4PC(tps []*types.FieldType) bool { | ||
if len(s) != len(tps) { | ||
return false | ||
} | ||
for i := range tps { | ||
// We only use part of logic of `func (ft *FieldType) Equal(other *FieldType)` here because (1) only numeric and | ||
// string types will show up here, and (2) we don't need flen and decimal to be matched exactly to use plan cache | ||
tpEqual := (s[i].GetType() == tps[i].GetType()) || | ||
(s[i].GetType() == mysql.TypeVarchar && tps[i].GetType() == mysql.TypeVarString) || | ||
(s[i].GetType() == mysql.TypeVarString && tps[i].GetType() == mysql.TypeVarchar) | ||
if !tpEqual || s[i].GetCharset() != tps[i].GetCharset() || s[i].GetCollate() != tps[i].GetCollate() || | ||
(s[i].EvalType() == types.ETInt && mysql.HasUnsignedFlag(s[i].GetFlag()) != mysql.HasUnsignedFlag(tps[i].GetFlag())) { | ||
return false | ||
} | ||
// When the type is decimal, we should compare the Flen and Decimal. | ||
// We can only use the plan when both Flen and Decimal should less equal than the cached one. | ||
// We assume here that there is no correctness problem when the precision of the parameters is less than the precision of the parameters in the cache. | ||
if tpEqual && s[i].GetType() == mysql.TypeNewDecimal && !(s[i].GetFlen() >= tps[i].GetFlen() && s[i].GetDecimal() >= tps[i].GetDecimal()) { | ||
return false | ||
} | ||
} | ||
return true | ||
} |
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.
Better to detach this function from FieldSlice
and implement it like checktypescompativility4PC(types1, types2 []*types.FieldType)
and put this new function in planner/core/plan_cache_utils.go
/
util/util.go
Outdated
@@ -283,3 +284,12 @@ func ReadLines(reader *bufio.Reader, count int, maxLineSize int) ([][]byte, erro | |||
} | |||
return lines, nil | |||
} | |||
|
|||
// PlanCacheMatchOpts store some property used to fetch plan from plan cache | |||
type PlanCacheMatchOpts struct { |
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.
Can you create a plancache package in util
? It can avoid unavailable cache for code import util package when updating type
code.
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.
Updated, PTAL
/retest |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: be428fc
|
/retest |
/merge |
/retest |
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
What problem does this PR solve?
Issue Number: ref #40219
Problem Summary:
What is changed and how it works?
No logical change
Gather the property which can used to get plan from LRU into a struct
PlanCacheMatchOpts
.We can add properties conveniently later
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.