Skip to content

Commit

Permalink
planner: remove the unnecessary skip-plan-cache flag in StmtCtx (ping…
Browse files Browse the repository at this point in the history
  • Loading branch information
ti-chi-bot authored Feb 15, 2023
1 parent 1c0e1e5 commit b85e280
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 14 deletions.
2 changes: 1 addition & 1 deletion expression/builtin_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 1 addition & 2 deletions expression/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion expression/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.False(t, ctx.GetSessionVars().StmtCtx.UseCache)
}

func TestEvaluateExprWithNullNoChangeRetType(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion expression/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions planner/core/plan_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 (
Expand Down
5 changes: 2 additions & 3 deletions sessionctx/stmtctx/stmtctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ type StatementContext struct {
InNullRejectCheck bool
AllowInvalidDate bool
IgnoreNoPartition bool
SkipPlanCache bool
IgnoreExplainIDSuffix bool
SkipUTF8Check bool
SkipASCIICheck bool
Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 2 additions & 4 deletions util/ranger/detacher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down

0 comments on commit b85e280

Please sign in to comment.