From f073ee053b5121f2aca636a42bebf8139e659bc8 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Fri, 30 Dec 2022 10:20:17 +0800 Subject: [PATCH 1/4] This is an automated cherry-pick of #40235 Signed-off-by: ti-chi-bot --- expression/builtin_compare.go | 2 +- expression/util.go | 2 +- planner/core/expression_rewriter.go | 5 +++++ planner/core/plan_cache.go | 4 ++-- sessionctx/stmtctx/stmtctx.go | 5 ++--- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index 4ffd0a3dbe6be..dec5d06983679 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -1586,7 +1586,7 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express } else { return args } - } else if ctx.GetSessionVars().StmtCtx.SkipPlanCache { + } else if !ctx.GetSessionVars().StmtCtx.UseCache { // We should remove the mutable constant for correctness, because its value may be changed. RemoveMutableConst(ctx, args) } diff --git a/expression/util.go b/expression/util.go index 22637d1b8617f..b7e66b97b24e8 100644 --- a/expression/util.go +++ b/expression/util.go @@ -1239,7 +1239,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.SkipPlanCache { + if !ctx.GetSessionVars().StmtCtx.UseCache { return false } return containMutableConst(ctx, exprs) diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index fcc81790cf074..570a030a626fd 100644 --- a/planner/core/expression_rewriter.go +++ b/planner/core/expression_rewriter.go @@ -1553,7 +1553,12 @@ func (er *expressionRewriter) inToExpression(lLen int, not bool, tp *types.Field if c.GetType().EvalType() == types.ETInt { continue // no need to refine it } +<<<<<<< HEAD er.sctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: '%v' may be converted to INT", c.String())) +======= + } else if !er.sctx.GetSessionVars().StmtCtx.UseCache { + // We should remove the mutable constant for correctness, because its value may be changed. +>>>>>>> ffaf2ac9eee (planner: remove the unnecessary skip-plan-cache flag in StmtCtx (#40235)) expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) } args[i], isExceptional = expression.RefineComparedConstant(er.sctx, *leftFt, c, opcode.EQ) diff --git a/planner/core/plan_cache.go b/planner/core/plan_cache.go index da6696d90ebd5..24f7af7cd115b 100644 --- a/planner/core/plan_cache.go +++ b/planner/core/plan_cache.go @@ -276,7 +276,7 @@ func generateNewPlan(ctx context.Context, sctx sessionctx.Context, isGeneralPlan if containTableDual(p) && paramNum > 0 { stmtCtx.SetSkipPlanCache(errors.New("skip plan-cache: get a TableDual plan")) } - if stmtAst.UseCache && !stmtCtx.SkipPlanCache && !ignorePlanCache { + if stmtCtx.UseCache && !ignorePlanCache { // rebuild key to exclude kv.TiFlash when stmt is not read only if _, isolationReadContainTiFlash := sessVars.IsolationReadEngines[kv.TiFlash]; isolationReadContainTiFlash && !IsReadOnly(stmtAst.Stmt, sessVars) { delete(sessVars.IsolationReadEngines, kv.TiFlash) @@ -636,7 +636,7 @@ func CheckPreparedPriv(sctx sessionctx.Context, stmt *PlanCacheStmt, is infosche // short paths for these executions, currently "point select" and "point update" func tryCachePointPlan(_ context.Context, sctx sessionctx.Context, stmt *PlanCacheStmt, _ infoschema.InfoSchema, p Plan) error { - if !sctx.GetSessionVars().StmtCtx.UseCache || sctx.GetSessionVars().StmtCtx.SkipPlanCache { + if !sctx.GetSessionVars().StmtCtx.UseCache { return nil } var ( diff --git a/sessionctx/stmtctx/stmtctx.go b/sessionctx/stmtctx/stmtctx.go index 59280cb5fb7e7..cdd5728d3ac54 100644 --- a/sessionctx/stmtctx/stmtctx.go +++ b/sessionctx/stmtctx/stmtctx.go @@ -171,7 +171,6 @@ type StatementContext struct { InNullRejectCheck bool AllowInvalidDate bool IgnoreNoPartition bool - SkipPlanCache bool IgnoreExplainIDSuffix bool SkipUTF8Check bool SkipASCIICheck bool @@ -599,10 +598,10 @@ func (sc *StatementContext) SetPlanHint(hint string) { // SetSkipPlanCache sets to skip the plan cache and records the reason. func (sc *StatementContext) SetSkipPlanCache(reason error) { - if sc.UseCache && sc.SkipPlanCache { + if !sc.UseCache { return // avoid unnecessary warnings } - sc.SkipPlanCache = true + sc.UseCache = false sc.AppendWarning(reason) } From e5c78bff52cb5457d790d0d90f9c9e1187c11c40 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 15 Feb 2023 15:30:11 +0800 Subject: [PATCH 2/4] fixup --- expression/expression_test.go | 2 +- planner/core/expression_rewriter.go | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/expression/expression_test.go b/expression/expression_test.go index e80cf5c41d575..63df451c98a37 100644 --- a/expression/expression_test.go +++ b/expression/expression_test.go @@ -77,7 +77,7 @@ func TestEvaluateExprWithNullAndParameters(t *testing.T) { res = EvaluateExprWithNull(ctx, schema, ltWithParam) _, isConst := res.(*Constant) require.True(t, isConst) // this expression is evaluated and skip-plan cache flag is set. - require.True(t, ctx.GetSessionVars().StmtCtx.SkipPlanCache) + require.True(t, ctx.GetSessionVars().StmtCtx.UseCache) } func TestEvaluateExprWithNullNoChangeRetType(t *testing.T) { diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index 570a030a626fd..fcc81790cf074 100644 --- a/planner/core/expression_rewriter.go +++ b/planner/core/expression_rewriter.go @@ -1553,12 +1553,7 @@ func (er *expressionRewriter) inToExpression(lLen int, not bool, tp *types.Field if c.GetType().EvalType() == types.ETInt { continue // no need to refine it } -<<<<<<< HEAD er.sctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: '%v' may be converted to INT", c.String())) -======= - } else if !er.sctx.GetSessionVars().StmtCtx.UseCache { - // We should remove the mutable constant for correctness, because its value may be changed. ->>>>>>> ffaf2ac9eee (planner: remove the unnecessary skip-plan-cache flag in StmtCtx (#40235)) expression.RemoveMutableConst(er.sctx, []expression.Expression{c}) } args[i], isExceptional = expression.RefineComparedConstant(er.sctx, *leftFt, c, opcode.EQ) From 91296fa8878d03aa2956d43ae3c5c8de0fd1eca9 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 15 Feb 2023 15:36:43 +0800 Subject: [PATCH 3/4] fixup --- expression/expression.go | 3 +-- util/ranger/detacher.go | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/expression/expression.go b/expression/expression.go index 3b4c7d87307c8..d1493aed92788 100644 --- a/expression/expression.go +++ b/expression/expression.go @@ -816,8 +816,7 @@ func SplitDNFItems(onExpr Expression) []Expression { // 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 MaybeOverOptimized4PlanCache(ctx, []Expression{expr}) { - ctx.GetSessionVars().StmtCtx.SkipPlanCache = true - ctx.GetSessionVars().StmtCtx.AppendWarning(errors.New("skip plan-cache: %v affects null check")) + ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.New("skip plan-cache: %v affects null check")) } if ctx.GetSessionVars().StmtCtx.InNullRejectCheck { expr, _ = evaluateExprWithNullInNullRejectCheck(ctx, schema, expr) diff --git a/util/ranger/detacher.go b/util/ranger/detacher.go index 4362f8ce63771..606f53c40265f 100644 --- a/util/ranger/detacher.go +++ b/util/ranger/detacher.go @@ -582,8 +582,7 @@ func ExtractEqAndInCondition(sctx sessionctx.Context, conditions []expression.Ex if len(points[offset]) == 0 { // Early termination if false expression found if expression.MaybeOverOptimized4PlanCache(sctx, conditions) { // `a>@x and a<@y` --> `invalid-range if @x>=@y` - sctx.GetSessionVars().StmtCtx.SkipPlanCache = true - sctx.GetSessionVars().StmtCtx.AppendWarning(errors.Errorf("skip plan-cache: some parameters may be overwritten")) + sctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: some parameters may be overwritten")) } return nil, nil, nil, nil, true } @@ -607,8 +606,7 @@ func ExtractEqAndInCondition(sctx sessionctx.Context, conditions []expression.Ex } else if len(points[i]) == 0 { // Early termination if false expression found if expression.MaybeOverOptimized4PlanCache(sctx, conditions) { // `a>@x and a<@y` --> `invalid-range if @x>=@y` - sctx.GetSessionVars().StmtCtx.SkipPlanCache = true - sctx.GetSessionVars().StmtCtx.AppendWarning(errors.Errorf("skip plan-cache: some parameters may be overwritten")) + sctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: some parameters may be overwritten")) } return nil, nil, nil, nil, true } else { From 98734efb12b94d4017f6ace03e2ce378b1946f0b Mon Sep 17 00:00:00 2001 From: qw4990 Date: Wed, 15 Feb 2023 16:21:53 +0800 Subject: [PATCH 4/4] fixup --- expression/expression_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/expression/expression_test.go b/expression/expression_test.go index 63df451c98a37..d9a352f74973f 100644 --- a/expression/expression_test.go +++ b/expression/expression_test.go @@ -77,7 +77,7 @@ func TestEvaluateExprWithNullAndParameters(t *testing.T) { res = EvaluateExprWithNull(ctx, schema, ltWithParam) _, isConst := res.(*Constant) require.True(t, isConst) // this expression is evaluated and skip-plan cache flag is set. - require.True(t, ctx.GetSessionVars().StmtCtx.UseCache) + require.False(t, ctx.GetSessionVars().StmtCtx.UseCache) } func TestEvaluateExprWithNullNoChangeRetType(t *testing.T) {