planner: fix bindings with ignore_plan_cache_hint cannot work #36427
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. |
ti-chi-bot
added
the
release-note-none
|
|
ti-chi-bot
added
the
size/M
|
Please follow PR Title Format:
Or if the count of mainly changed packages are more than 3, use
After you have format title, you can leave a comment |
|
/retitle planner:Fix bindings with ignore_plan_cache_hint cannot work |
ti-chi-bot
changed the title
|
/run-check_title |
|
Please follow PR Title Format:
Or if the count of mainly changed packages are more than 3, use
After you have format title, you can leave a comment |
|
/retitle planner: fix bindings with ignore_plan_cache_hint cannot work |
ti-chi-bot
changed the title
|
/run-check_title |
|
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/1e2d6de1b21e0b3be8568de9b5496d236bdb0082 |
|
/cc @qw4990 @Reminiscent |
|
/run-unit-test |
|
/run-check_dev_2 |
planner/core/common_plans.go
Outdated
| @@ -318,7 +318,7 @@ func (e *Execute) checkPreparedPriv(ctx context.Context, sctx sessionctx.Context | |||
| } | |||
|
|
|||
| // GetBindSQL4PlanCache used to get the bindSQL for plan cache to build the plan cache key. | |||
| func GetBindSQL4PlanCache(sctx sessionctx.Context, preparedStmt *CachedPrepareStmt) string { | |||
| func GetBindSQL4PlanCache(sctx sessionctx.Context, preparedStmt *CachedPrepareStmt, ignore *bool) string { | |||
There was a problem hiding this comment.
Why not treat the ignore as the return value?
There was a problem hiding this comment.
thks, i'm adapting golang‘s idiomatic syntax
| @@ -1174,6 +1174,31 @@ func TestHint4PlanCache(t *testing.T) { | |||
| tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("0")) | |||
| } | |||
|
|
|||
| func TestIgnorePlanCacheWithPrepare(t *testing.T) { | |||
There was a problem hiding this comment.
Please add some test cases like:
create binding for ... using select /*+ use_index(..) */ .. // some useful hint
execute stmt;
select @@last_plan_from_binding; // check binding
execute stmt;
select @@last_plan_from_cache; // check the plan cache
create binding for ... using select /*+ ignore_plan_cache() */ ..
execute;
select @@last_plan_from_cache;
execute;
select @@last_plan_from_binding;
There was a problem hiding this comment.
@Reminiscent PTAL agian and by the way Am I putting UnitTest code in the right place(*.go)?
add some test cases and modify a function
| tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("0")) | ||
| tk.MustQuery("execute stmt;").Check(testkit.Rows()) | ||
| tk.MustQuery("select @@last_plan_from_binding;").Check(testkit.Rows("1")) | ||
|
|
There was a problem hiding this comment.
Please add one more test case like:
create binding for ... using select /*+ use_index(t, idx_a) */ ... -- remove the ignore_plan_cahce hint
execute stmt;
execute stmt;
select @@last_plan_from_cache; -- the plan cache can work again
select @@last_plan_from_binding;
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 6135e5a
|
ti-chi-bot
added
the
status/can-merge
|
/run-unit-test |
|
/retitle planner: fix bindings with ignore_plan_cache() hint cannot work |
TiDB MergeCI notify🔴 Bad News! New failing [1] after this pr merged.
|
jebter
added
the
needs-cherry-pick-release-6.1
Signed-off-by: ti-srebot <[email protected]>
|
cherry pick to release-6.1 in PR #37231 |
What problem does this PR solve?
Issue Number: close #34596
Problem Summary:
In the general SQL, using hint
/*+ ignore_plan_cache() */cannot be awared by prepare/execute.What is changed and how it works?
Add some check in EXECUTE stage.
The switch of prepare's plan cache is open/close in PREPARE stage.But if we CREATE/DROP binding with
/*+ ignore_plan_cache() */, the switch will not change in EXECUTE stage.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.