planner: Fix rebuild range for prepared plan

[blacktear23]

What problem does this PR solve?

Issue Number: close #23304 close #23290

Problem Summary:

What is changed and how it works?

What's Changed:
Rebuild ranges for the prepared plan may get the wrong result. This PR will fix the BatchPointGet plan's Handle array to fix newly variables bind and will try to convert BatchPointGet to PointGet when newly variables will only have one handle, and it also can transform PointGet convert to BatchPointGet when variables may generate more than 1 handle.

How it Works:

Related changes

• Need to cherry-pick to the release branch

Check List

Tests

• Unit test
• Integration test

Side effects

• Performance regression
  • Consumes more CPU
  • Consumes more MEM

Release note

• Fix prepared plan will get wrong result when variables changed.

[sre-bot]

Please follow PR Title Format:

• pkg [, pkg2, pkg3]: what's changed

Or if the count of mainly changed packages are more than 3, use

• *: what's changed

[zz-jason]

we need to cherry-pick this critical bug-fix to release 3.0, 4.0, and 5.0

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• winoros

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

[blacktear23]

@zz-jason this PR will not works with master branch. But it may work in 4.0 branch. The root cause for fail at master branch still need investigating.

[blacktear23]

If comments

tidb/planner/core/common_plans.go

Line 413 in e56ef47

case *PointGetPlan:

case and below codes. Then PointGetPlan will not cached, this PR will work correctly. And seems #23290 will also be fixed. @bb7133

[XuHuaiyu]

If this commit can also fix #23290, we may also add this issue number in the PR description. @blacktear23

[blacktear23]

@zz-jason After review release-4.0 branch, the code is not same as current master, I think we should create a new PR for other release branches. And I found the code for special process PointGetPlan cache maybe the base cause for those Bugs. The cached PointGetPlan cannot update AccessConditions, so it will generate wrong ranges when variables changes. So I think disable special PointGetPlan cache logic maybe the best way to fix.

And I found UseCache is always false for release-4.0 and master branch. This can disable other plan cache, but it cannot disable PointGetPlan special cache. And I found in some day before 2021-1-12 it switch to true but then some day after 2021-1-12 it turn to false again.

[eurekaka]

I think #23238 can fix both #23304 and #23290 as well.

[blacktear23]

@eurekaka After read your PR, it will fix not cache PointGetPlan, but if enable prepared plan cache and BatchPointGet plan got cached, I think rebuildRange function also has bugs on Handles array rebuild.

[eurekaka]

@eurekaka After read your PR, it will fix not cache PointGetPlan, but if enable prepared plan cache and BatchPointGet plan got cached, I think rebuildRange function also has bugs on Handles array rebuild.

Could you please manufacture a test case that would fail after applying #23238 ?

[blacktear23]

@eurekaka After read your PR, it will fix not cache PointGetPlan, but if enable prepared plan cache and BatchPointGet plan got cached, I think rebuildRange function also has bugs on Handles array rebuild.

Could you please manufacture a test case that would fail after applying #23238 ?

After I do the test on your feature branch, yes bugs cannot reproduce now. But your PR just disables PreparedPlanCache for statements with variables. Is that what we expected?

[eurekaka]

just disables PreparedPlanCache for statements with variables

How does this come? #23238 prevents plans with aggressive optimizations from being cached, because those aggressive optimizations may depend on the user variables.

[blacktear23]

just disables PreparedPlanCache for statements with variables

How does this come? #23238 prevents plans with aggressive optimizations from being cached, because those aggressive optimizations may depend on the user variables.

In my test I see OptimDependOnMutableConst is always true, so even BatchPointGet plan still cannot be cached. Or is there has any way to let BatchPointGet can be cached?

Anyway the rebuildRanges logic is not correct I think we should fix the logic even PR #23238 can fix those issues.

[zz-jason]

I'm going to close this PR since it hasn't been updated for a long time. Feel free to reopen it once you wish to advance this PR in the future.