From caad839aebf2029714df09faddadbadf41c74e63 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Tue, 15 Mar 2022 19:13:52 +0800 Subject: [PATCH] planner: fix the wrong range built for bit columns when reusing cached plan (#33090) close pingcap/tidb#33067 --- expression/builtin_other.go | 7 +++++-- planner/core/common_plans.go | 12 ++++++++++-- planner/core/prepare_test.go | 24 ++++++++++++++++++++++++ sessionctx/stmtctx/stmtctx.go | 1 + util/ranger/ranger.go | 4 ++++ 5 files changed, 44 insertions(+), 4 deletions(-) diff --git a/expression/builtin_other.go b/expression/builtin_other.go index 12b372bde63a6..fa4b7d6485bb3 100644 --- a/expression/builtin_other.go +++ b/expression/builtin_other.go @@ -85,7 +85,7 @@ type inFunctionClass struct { } func (c *inFunctionClass) getFunction(ctx sessionctx.Context, args []Expression) (sig builtinFunc, err error) { - args, err = c.verifyArgs(args) + args, err = c.verifyArgs(ctx, args) if err != nil { return nil, err } @@ -157,7 +157,7 @@ func (c *inFunctionClass) getFunction(ctx sessionctx.Context, args []Expression) return sig, nil } -func (c *inFunctionClass) verifyArgs(args []Expression) ([]Expression, error) { +func (c *inFunctionClass) verifyArgs(ctx sessionctx.Context, args []Expression) ([]Expression, error) { columnType := args[0].GetType() validatedArgs := make([]Expression, 0, len(args)) for _, arg := range args { @@ -165,6 +165,9 @@ func (c *inFunctionClass) verifyArgs(args []Expression) ([]Expression, error) { switch { case columnType.Tp == mysql.TypeBit && constant.Value.Kind() == types.KindInt64: if constant.Value.GetInt64() < 0 { + if MaybeOverOptimized4PlanCache(ctx, args) { + ctx.GetSessionVars().StmtCtx.SkipPlanCache = true + } continue } } diff --git a/planner/core/common_plans.go b/planner/core/common_plans.go index 838a629175bb4..fd6e1ef717798 100644 --- a/planner/core/common_plans.go +++ b/planner/core/common_plans.go @@ -454,7 +454,7 @@ func (e *Execute) getPhysicalPlan(ctx context.Context, sctx sessionctx.Context, // so you don't need to consider whether prepared.useCache is enabled. plan := prepared.CachedPlan.(Plan) names := prepared.CachedNames.(types.NameSlice) - err := e.rebuildRange(plan) + err := e.RebuildPlan(plan) if err != nil { logutil.BgLogger().Debug("rebuild range failed", zap.Error(err)) goto REBUILD @@ -501,7 +501,7 @@ func (e *Execute) getPhysicalPlan(ctx context.Context, sctx sessionctx.Context, } } if planValid { - err := e.rebuildRange(cachedVal.Plan) + err := e.RebuildPlan(cachedVal.Plan) if err != nil { logutil.BgLogger().Debug("rebuild range failed", zap.Error(err)) goto REBUILD @@ -629,6 +629,14 @@ func (e *Execute) tryCachePointPlan(ctx context.Context, sctx sessionctx.Context return err } +// RebuildPlan will rebuild this plan under current user parameters. +func (e *Execute) RebuildPlan(p Plan) error { + sc := p.SCtx().GetSessionVars().StmtCtx + sc.InPreparedPlanBuilding = true + defer func() { sc.InPreparedPlanBuilding = false }() + return e.rebuildRange(p) +} + func (e *Execute) rebuildRange(p Plan) error { sctx := p.SCtx() sc := p.SCtx().GetSessionVars().StmtCtx diff --git a/planner/core/prepare_test.go b/planner/core/prepare_test.go index 295ae2036c521..3675aee5b5bc0 100644 --- a/planner/core/prepare_test.go +++ b/planner/core/prepare_test.go @@ -1582,6 +1582,30 @@ func TestIssue28254(t *testing.T) { tk.MustQuery("execute stmt using @a").Check(testkit.Rows("1")) } +func TestIssue33067(t *testing.T) { + store, clean := testkit.CreateMockStore(t) + defer clean() + orgEnable := core.PreparedPlanCacheEnabled() + defer core.SetPreparedPlanCache(orgEnable) + core.SetPreparedPlanCache(true) + se, err := session.CreateSession4TestWithOpt(store, &session.Opt{ + PreparedPlanCache: kvcache.NewSimpleLRUCache(100, 0.1, math.MaxUint64), + }) + require.NoError(t, err) + tk := testkit.NewTestKitWithSession(t, store, se) + + tk.MustExec("use test") + tk.MustExec("DROP TABLE IF EXISTS `t`") + tk.MustExec("CREATE TABLE `t` (`COL1` char(20) DEFAULT NULL, `COL2` bit(16),`COL3` date, KEY `U_M_COL5` (`COL3`,`COL2`))") + tk.MustExec("insert into t values ('','>d','9901-06-17')") + tk.MustExec("prepare stmt from 'select * from t where col1 is not null and col2 not in (?, ?, ?) and col3 in (?, ?, ?)'") + tk.MustExec(`set @a=-21188, @b=26824, @c=31855, @d="5597-1-4", @e="5755-12-6", @f="1253-7-12"`) + tk.MustQuery(`execute stmt using @a,@b,@c,@d,@e,@f`).Check(testkit.Rows()) + tk.MustExec(`set @a=-5360, @b=-11715, @c=9399, @d="9213-09-13", @e="4705-12-24", @f="9901-06-17"`) + tk.MustQuery(`execute stmt using @a,@b,@c,@d,@e,@f`).Check(testkit.Rows(" >d 9901-06-17")) + tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) +} + func TestIssue29486(t *testing.T) { store, clean := testkit.CreateMockStore(t) defer clean() diff --git a/sessionctx/stmtctx/stmtctx.go b/sessionctx/stmtctx/stmtctx.go index ce8f10c875f8f..fcc166141e6ff 100644 --- a/sessionctx/stmtctx/stmtctx.go +++ b/sessionctx/stmtctx/stmtctx.go @@ -75,6 +75,7 @@ type StatementContext struct { InLoadDataStmt bool InExplainStmt bool InCreateOrAlterStmt bool + InPreparedPlanBuilding bool IgnoreTruncate bool IgnoreZeroInDate bool NoZeroDate bool diff --git a/util/ranger/ranger.go b/util/ranger/ranger.go index 0e5e9988062b3..dc1d439d0f3c6 100644 --- a/util/ranger/ranger.go +++ b/util/ranger/ranger.go @@ -101,6 +101,10 @@ func convertPoint(sctx sessionctx.Context, point *point, tp *types.FieldType) (* } casted, err := point.value.ConvertTo(sc, tp) if err != nil { + if sctx.GetSessionVars().StmtCtx.InPreparedPlanBuilding { + // do not ignore these errors if in prepared plan building for safety + return nil, errors.Trace(err) + } if tp.Tp == mysql.TypeYear && terror.ErrorEqual(err, types.ErrWarnDataOutOfRange) { // see issue #20101: overflow when converting integer to year } else if tp.Tp == mysql.TypeBit && terror.ErrorEqual(err, types.ErrDataTooLong) {