-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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: handle PointPlan specially in execute statements #36987
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. |
planner/core/plan_cache.go
Outdated
// IsPointPlanShortPathOK check if we can execute using plan cached in prepared structure | ||
// Be careful with the short path, current precondition is ths cached plan satisfying | ||
// IsPointGetWithPKOrUniqueKeyByAutoCommit | ||
func IsPointPlanShortPathOK(sctx sessionctx.Context, preparedStmt *CachedPrepareStmt) (bool, error) { |
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.
session/session.go
Outdated
if err != nil { | ||
return nil, err | ||
} | ||
if pointShortPathOK { |
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.
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.
I remembered that case nil
is added intentionly, Reminiscent@92b1b8e
Safe to remove?
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.
We can ignore this case-branch since runStmt
is used by default.
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/2c1882d3bd50b7ca2476c81e02b84deac60adb78 |
65bffdd
to
1830bdc
Compare
Master v.s. this PR:
|
/merge |
This pull request has been accepted and is ready to merge. Commit hash: d5da263
|
/hold @qw4990 Please fix the unit test. |
/unhold |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: cba829c
|
TiDB MergeCI notify🔴 Bad News! New failing [3] after this pr merged.
|
What problem does this PR solve?
Issue Number: close #36783
Problem Summary: planner: handle PointPlan specially in execute statements
What is changed and how it works?
Following #36587, handle PointPlan specially in execute statements to gain some performance improvement.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.