From 255e59f52f148bcb4351ccdb6131454ca46731fb Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Mon, 15 Nov 2021 15:43:13 +0800 Subject: [PATCH 1/4] planner: make clear for MaybeOverOptimized4PlanCache --- expression/builtin_compare.go | 2 ++ expression/util.go | 7 +------ planner/core/expression_rewriter.go | 2 ++ 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index dce8d2cdd3ba0..ff25b346e3221 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -1465,6 +1465,8 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express } else { return args } + } else if ctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { + RemoveMutableConst(ctx, args) } // int non-constant [cmp] non-int constant if arg0IsInt && !arg0IsCon && !arg1IsInt && arg1IsCon { diff --git a/expression/util.go b/expression/util.go index 43af814cc0742..d2f04d58d3a95 100644 --- a/expression/util.go +++ b/expression/util.go @@ -920,12 +920,7 @@ func ContainCorrelatedColumn(exprs []Expression) bool { // TODO: Do more careful check here. func MaybeOverOptimized4PlanCache(ctx sessionctx.Context, exprs []Expression) bool { // If we do not enable plan cache, all the optimization can work correctly. - if !ctx.GetSessionVars().StmtCtx.UseCache { - return false - } - if ctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { - // If the current statement can not be cached. We should remove the mutable constant. - RemoveMutableConst(ctx, exprs) + if !ctx.GetSessionVars().StmtCtx.UseCache || ctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { return false } return containMutableConst(ctx, exprs) diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index b2c270b923431..05d61519d6176 100644 --- a/planner/core/expression_rewriter.go +++ b/planner/core/expression_rewriter.go @@ -1459,6 +1459,8 @@ func (er *expressionRewriter) inToExpression(lLen int, not bool, tp *types.Field } else { continue } + } else if er.sctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { + expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) } args[i], isExceptional = expression.RefineComparedConstant(er.sctx, *leftFt, c, opcode.EQ) if isExceptional { From ad5f8f941f9f73ef4872a358ee6340fa1229f480 Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Wed, 17 Nov 2021 11:00:46 +0800 Subject: [PATCH 2/4] add more comments --- expression/builtin_compare.go | 1 + planner/core/expression_rewriter.go | 1 + 2 files changed, 2 insertions(+) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index ff25b346e3221..cfd258b40d9aa 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -1466,6 +1466,7 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express return args } } else if ctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { + // We should remove the mutable constant for correctness, because it's value maybe changed. RemoveMutableConst(ctx, args) } // int non-constant [cmp] non-int constant diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index 05d61519d6176..4126c22d54ed0 100644 --- a/planner/core/expression_rewriter.go +++ b/planner/core/expression_rewriter.go @@ -1460,6 +1460,7 @@ func (er *expressionRewriter) inToExpression(lLen int, not bool, tp *types.Field continue } } else if er.sctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { + // We should remove the mutable constant for correctness, because it's value maybe changed. expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) } args[i], isExceptional = expression.RefineComparedConstant(er.sctx, *leftFt, c, opcode.EQ) From d3f2c3c73968d06c52fbb71a01bd53f3928a5308 Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Mon, 22 Nov 2021 11:59:05 +0800 Subject: [PATCH 3/4] address comments --- expression/builtin_compare.go | 6 ++-- expression/util.go | 2 +- planner/core/common_plans.go | 4 +-- planner/core/expression_rewriter.go | 6 ++-- planner/core/optimizer.go | 2 +- sessionctx/stmtctx/stmtctx.go | 46 ++++++++++++++--------------- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index cfd258b40d9aa..948199aea7770 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -1460,13 +1460,13 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express // To keep the result be compatible with MySQL, refine `int non-constant str constant` // here and skip this refine operation in all other cases for safety. if (arg0IsInt && !arg0IsCon && arg1IsString && arg1IsCon) || (arg1IsInt && !arg1IsCon && arg0IsString && arg0IsCon) { - ctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache = true + ctx.GetSessionVars().StmtCtx.SkipPlanCache = true RemoveMutableConst(ctx, args) } else { return args } - } else if ctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { - // We should remove the mutable constant for correctness, because it's value maybe changed. + } else if ctx.GetSessionVars().StmtCtx.SkipPlanCache { + // We should remove the mutable constant for correctness, because its value may be changed. RemoveMutableConst(ctx, args) } // int non-constant [cmp] non-int constant diff --git a/expression/util.go b/expression/util.go index d2f04d58d3a95..7b34ef442067b 100644 --- a/expression/util.go +++ b/expression/util.go @@ -920,7 +920,7 @@ func ContainCorrelatedColumn(exprs []Expression) bool { // TODO: Do more careful check here. func MaybeOverOptimized4PlanCache(ctx sessionctx.Context, exprs []Expression) bool { // If we do not enable plan cache, all the optimization can work correctly. - if !ctx.GetSessionVars().StmtCtx.UseCache || ctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { + if !ctx.GetSessionVars().StmtCtx.UseCache || ctx.GetSessionVars().StmtCtx.SkipPlanCache { return false } return containMutableConst(ctx, exprs) diff --git a/planner/core/common_plans.go b/planner/core/common_plans.go index d69f0fa05d761..dec06e088ade3 100644 --- a/planner/core/common_plans.go +++ b/planner/core/common_plans.go @@ -481,7 +481,7 @@ REBUILD: e.names = names e.Plan = p _, isTableDual := p.(*PhysicalTableDual) - if !isTableDual && prepared.UseCache && !stmtCtx.MaybeOverOptimized4PlanCache { + if !isTableDual && prepared.UseCache && !stmtCtx.SkipPlanCache { // rebuild key to exclude kv.TiFlash when stmt is not read only if _, isolationReadContainTiFlash := sessVars.IsolationReadEngines[kv.TiFlash]; isolationReadContainTiFlash && !IsReadOnly(stmt, sessVars) { delete(sessVars.IsolationReadEngines, kv.TiFlash) @@ -516,7 +516,7 @@ REBUILD: // short paths for these executions, currently "point select" and "point update" func (e *Execute) tryCachePointPlan(ctx context.Context, sctx sessionctx.Context, preparedStmt *CachedPrepareStmt, is infoschema.InfoSchema, p Plan) error { - if sctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { + if sctx.GetSessionVars().StmtCtx.SkipPlanCache { return nil } var ( diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index 4126c22d54ed0..84c45730baa65 100644 --- a/planner/core/expression_rewriter.go +++ b/planner/core/expression_rewriter.go @@ -1454,13 +1454,13 @@ func (er *expressionRewriter) inToExpression(lLen int, not bool, tp *types.Field if c.GetType().EvalType() == types.ETString { // To keep the result be compatible with MySQL, refine `int non-constant str constant` // here and skip this refine operation in all other cases for safety. - er.sctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache = true + er.sctx.GetSessionVars().StmtCtx.SkipPlanCache = true expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) } else { continue } - } else if er.sctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache { - // We should remove the mutable constant for correctness, because it's value maybe changed. + } else if er.sctx.GetSessionVars().StmtCtx.SkipPlanCache { + // We should remove the mutable constant for correctness, because its value may be changed. expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) } args[i], isExceptional = expression.RefineComparedConstant(er.sctx, *leftFt, c, opcode.EQ) diff --git a/planner/core/optimizer.go b/planner/core/optimizer.go index 55127627b7477..204859252f988 100644 --- a/planner/core/optimizer.go +++ b/planner/core/optimizer.go @@ -316,7 +316,7 @@ func postOptimize(sctx sessionctx.Context, plan PhysicalPlan) PhysicalPlan { // Todo: make more careful check here. func checkPlanCacheable(sctx sessionctx.Context, plan PhysicalPlan) { if sctx.GetSessionVars().StmtCtx.UseCache && useTiFlash(plan) { - sctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache = true + sctx.GetSessionVars().StmtCtx.SkipPlanCache = true } } diff --git a/sessionctx/stmtctx/stmtctx.go b/sessionctx/stmtctx/stmtctx.go index 03c488c883a0c..725798da421cc 100644 --- a/sessionctx/stmtctx/stmtctx.go +++ b/sessionctx/stmtctx/stmtctx.go @@ -65,29 +65,29 @@ type StatementContext struct { // IsDDLJobInQueue is used to mark whether the DDL job is put into the queue. // If IsDDLJobInQueue is true, it means the DDL job is in the queue of storage, and it can be handled by the DDL worker. - IsDDLJobInQueue bool - InInsertStmt bool - InUpdateStmt bool - InDeleteStmt bool - InSelectStmt bool - InLoadDataStmt bool - InExplainStmt bool - InCreateOrAlterStmt bool - IgnoreTruncate bool - IgnoreZeroInDate bool - DupKeyAsWarning bool - BadNullAsWarning bool - DividedByZeroAsWarning bool - TruncateAsWarning bool - OverflowAsWarning bool - InShowWarning bool - UseCache bool - BatchCheck bool - InNullRejectCheck bool - AllowInvalidDate bool - IgnoreNoPartition bool - MaybeOverOptimized4PlanCache bool - IgnoreExplainIDSuffix bool + IsDDLJobInQueue bool + InInsertStmt bool + InUpdateStmt bool + InDeleteStmt bool + InSelectStmt bool + InLoadDataStmt bool + InExplainStmt bool + InCreateOrAlterStmt bool + IgnoreTruncate bool + IgnoreZeroInDate bool + DupKeyAsWarning bool + BadNullAsWarning bool + DividedByZeroAsWarning bool + TruncateAsWarning bool + OverflowAsWarning bool + InShowWarning bool + UseCache bool + BatchCheck bool + InNullRejectCheck bool + AllowInvalidDate bool + IgnoreNoPartition bool + SkipPlanCache bool + IgnoreExplainIDSuffix bool // If the select statement was like 'select * from t as of timestamp ...' or in a stale read transaction // or is affected by the tidb_read_staleness session variable, then the statement will be makred as isStaleness // in stmtCtx From 28ac7b21e3d372b7103304f243e7ff81900aab81 Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Thu, 25 Nov 2021 15:22:20 +0800 Subject: [PATCH 4/4] fix ut --- planner/core/find_best_task.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/find_best_task.go b/planner/core/find_best_task.go index e0abd33d197b7..baaf5d17f483c 100644 --- a/planner/core/find_best_task.go +++ b/planner/core/find_best_task.go @@ -806,7 +806,7 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter if len(path.Ranges) == 0 { // We should uncache the tableDual plan. if expression.MaybeOverOptimized4PlanCache(ds.ctx, path.AccessConds) { - ds.ctx.GetSessionVars().StmtCtx.MaybeOverOptimized4PlanCache = true + ds.ctx.GetSessionVars().StmtCtx.SkipPlanCache = true } dual := PhysicalTableDual{}.Init(ds.ctx, ds.stats, ds.blockOffset) dual.SetSchema(ds.schema)