planner: fix the issue that some PointGet plans generated in physical-stage cannot be cached #28185
Conversation
qw4990
added
type/enhancement
|
[REVIEW NOTIFICATION] This pull request has not been approved. 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
release-note
| @@ -789,8 +789,8 @@ func SplitDNFItems(onExpr Expression) []Expression { | |||
| // EvaluateExprWithNull sets columns in schema as null and calculate the final result of the scalar function. | |||
| // If the Expression is a non-constant value, it means the result is unknown. | |||
| func EvaluateExprWithNull(ctx sessionctx.Context, schema *Schema, expr Expression) Expression { | |||
| if ContainMutableConst(ctx, []Expression{expr}) { | |||
| ctx.GetSessionVars().StmtCtx.OptimDependOnMutableConst = true | |||
| if ctx.GetSessionVars().StmtCtx.UseCache && ContainMutableConst(ctx, []Expression{expr}) { | |||
There was a problem hiding this comment.
The function ContainMutableConst contains the check for ctx.GetSessionVars().StmtCtx.UseCache.
| @@ -1107,7 +1149,7 @@ func (s *testPlanSerialSuite) TestPlanCachePointGetAndTableDual(c *C) { | |||
| tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) | |||
| // Must not reuse the previous TableDual plan. | |||
There was a problem hiding this comment.
Please modify the comment here. Besides, we need to check the plan from the cache. The following changes are the same.
| @@ -604,7 +602,7 @@ func (ds *DataSource) isInIndexMergeHints(name string) bool { | |||
| } | |||
|
|
|||
| // accessPathsForConds generates all possible index paths for conditions. | |||
There was a problem hiding this comment.
Please update the comment here.
| // TODO: Can we make a more careful check on whether the optimization depends on mutable constants? | ||
| ds.ctx.GetSessionVars().StmtCtx.OptimDependOnMutableConst = true |
There was a problem hiding this comment.
Are there some test cases that can cover this change?
| @@ -566,7 +566,9 @@ func ExtractEqAndInCondition(sctx sessionctx.Context, conditions []expression.Ex | |||
| // Maybe we can improve it later. | |||
| columnValues[i] = &valueInfo{mutable: true} | |||
| } | |||
| sctx.GetSessionVars().StmtCtx.OptimDependOnMutableConst = true | |||
| if sctx.GetSessionVars().StmtCtx.UseCache { | |||
There was a problem hiding this comment.
If the prepare stmt like prepare stmt for 'select * from t where a > 0 and a < 2' which there are no parameters, we should use the PointGet plan. But here we can't use this plan?
There was a problem hiding this comment.
In real workloads there may be a small chance to convert a > ? and a < ? to equal condition, so maybe it is acceptable to give up the optimization when prepare-plan-cache is enabled.
|
@qw4990: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ti-chi-bot
added
the
needs-rebase
| canConvertPointGet := len(path.Ranges) > 0 && path.StoreType == kv.TiKV && ds.isPointGetConvertableSchema() && | ||
| // to avoid the over-optimized risk, do not generate PointGet for plan cache, for example, | ||
| // `pk>=$a and pk<=$b` can be optimized to a PointGet when `$a==$b`, but it can cause wrong results when `$a!=$b`. | ||
| !ds.ctx.GetSessionVars().StmtCtx.UseCache |
There was a problem hiding this comment.
Here we cannot generate PointGet if prepared-cache-plan is enabled. Will that be a problem in OLTP workload? For example, as for prepare stmt1 from "insert into t2 select * from t1 where a=?", if the sql will be executed frequently, we may hope to not only generate PointGet plan but also cache the plan.
|
There are too many conflicts, I'll close this PR and submit a new one later. |
What problem does this PR solve?
Issue Number: close #26868
Problem Summary: planner: fix the issue that some PointGet plans generated in physical-stage cannot be cached
What is changed and how it works?
planner: fix the issue that some PointGet plans generated in physical-stage cannot be cached
Check List
Tests
Release note