From 23e4547f30ed8892942049297174645936efe90f Mon Sep 17 00:00:00 2001 From: Chengpeng Yan <41809508+Reminiscent@users.noreply.github.com> Date: Tue, 29 Mar 2022 13:56:28 +0800 Subject: [PATCH] cherry pick #31769 to release-5.4 Signed-off-by: ti-srebot --- expression/builtin_arithmetic.go | 5 ----- planner/core/cache.go | 13 ++++++++++--- planner/core/common_plans.go | 4 ++-- planner/core/prepare_test.go | 26 ++++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/expression/builtin_arithmetic.go b/expression/builtin_arithmetic.go index 6c82a938206ce..38db160b6c699 100644 --- a/expression/builtin_arithmetic.go +++ b/expression/builtin_arithmetic.go @@ -90,11 +90,6 @@ func numericContextResultType(ft *types.FieldType) types.EvalType { // type according to the two input parameter's types. func setFlenDecimal4RealOrDecimal(ctx sessionctx.Context, retTp *types.FieldType, arg0, arg1 Expression, isReal bool, isMultiply bool) { a, b := arg0.GetType(), arg1.GetType() - if MaybeOverOptimized4PlanCache(ctx, []Expression{arg0, arg1}) { - // set length and decimal to unspecified if arguments depend on parameters - retTp.Flen, retTp.Decimal = types.UnspecifiedLength, types.UnspecifiedLength - return - } if a.Decimal != types.UnspecifiedLength && b.Decimal != types.UnspecifiedLength { retTp.Decimal = a.Decimal + b.Decimal if !isMultiply { diff --git a/planner/core/cache.go b/planner/core/cache.go index 0d01f2bb98f73..388235a72adfb 100644 --- a/planner/core/cache.go +++ b/planner/core/cache.go @@ -154,9 +154,10 @@ func NewPlanCacheKey(sessionVars *variable.SessionVars, pstmtID uint32, schemaVe // FieldSlice is the slice of the types.FieldType type FieldSlice []types.FieldType -// Equal compares FieldSlice with []*types.FieldType -// Currently this is only used in plan cache to invalidate cache when types of variables are different. -func (s FieldSlice) Equal(tps []*types.FieldType) bool { +// CheckTypesCompatibility4PC compares FieldSlice with []*types.FieldType +// Currently this is only used in plan cache to check whether the types of parameters are compatible. +// If the types of parameters are compatible, we can use the cached plan. +func (s FieldSlice) CheckTypesCompatibility4PC(tps []*types.FieldType) bool { if len(s) != len(tps) { return false } @@ -172,6 +173,12 @@ func (s FieldSlice) Equal(tps []*types.FieldType) bool { (s[i].EvalType() == types.ETInt && mysql.HasUnsignedFlag(s[i].Flag) != mysql.HasUnsignedFlag(tps[i].Flag)) { return false } + // When the type is decimal, we should compare the Flen and Decimal. + // We can only use the plan when both Flen and Decimal should less equal than the cached one. + // We assume here that there is no correctness problem when the precision of the parameters is less than the precision of the parameters in the cache. + if tpEqual && s[i].Tp == mysql.TypeNewDecimal && !(s[i].Flen >= tps[i].Flen && s[i].Decimal >= tps[i].Decimal) { + return false + } } return true } diff --git a/planner/core/common_plans.go b/planner/core/common_plans.go index 39d361ee4045c..7c1e3b130db4d 100644 --- a/planner/core/common_plans.go +++ b/planner/core/common_plans.go @@ -497,7 +497,7 @@ func (e *Execute) getPhysicalPlan(ctx context.Context, sctx sessionctx.Context, sctx.PreparedPlanCache().Delete(cacheKey) break } - if !cachedVal.UserVarTypes.Equal(tps) { + if !cachedVal.UserVarTypes.CheckTypesCompatibility4PC(tps) { continue } planValid := true @@ -572,7 +572,7 @@ REBUILD: if cacheVals, exists := sctx.PreparedPlanCache().Get(cacheKey); exists { hitVal := false for i, cacheVal := range cacheVals.([]*PlanCacheValue) { - if cacheVal.UserVarTypes.Equal(tps) { + if cacheVal.UserVarTypes.CheckTypesCompatibility4PC(tps) { hitVal = true cacheVals.([]*PlanCacheValue)[i] = cached break diff --git a/planner/core/prepare_test.go b/planner/core/prepare_test.go index 6b336384ef8da..e4793424890c6 100644 --- a/planner/core/prepare_test.go +++ b/planner/core/prepare_test.go @@ -1733,9 +1733,13 @@ func (s *testPlanSerialSuite) TestIssue29565(c *C) { tk.MustQuery(`execute stmt using @a,@b`).Check(testkit.Rows()) tk.MustExec(`set @a=5408499810319315618, @b=-9999999999999999999999999999999999999999999999999999999`) tk.MustQuery(`execute stmt using @a,@b`).Check(testkit.Rows("-9999999999999999999999999999999999999999999999999999999")) + tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) + tk.MustExec(`set @a=7309027171262036496, @b=-9798213896406520625`) + tk.MustQuery(`execute stmt using @a,@b`).Check(testkit.Rows()) tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("1")) } +<<<<<<< HEAD func (s *testPlanSerialSuite) TestIssue28828(c *C) { store, dom, err := newStoreWithBootstrap() c.Assert(err, IsNil) @@ -1744,6 +1748,28 @@ func (s *testPlanSerialSuite) TestIssue28828(c *C) { dom.Close() store.Close() }() +======= +func TestIssue31730(t *testing.T) { + store, clean := testkit.CreateMockStore(t) + defer clean() + orgEnable := core.PreparedPlanCacheEnabled() + defer core.SetPreparedPlanCache(orgEnable) + core.SetPreparedPlanCache(true) + tk := testkit.NewTestKit(t, store) + + tk.MustExec(`use test`) + tk.MustExec(`drop table if exists PK_S_MULTI_37;`) + tk.MustExec(`CREATE TABLE PK_S_MULTI_37 (COL1 decimal(55,0) NOT NULL, COL2 decimal(55,0) NOT NULL,PRIMARY KEY (COL1, COL2) /*T![clustered_index] NONCLUSTERED */) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;`) + tk.MustExec(`insert into PK_S_MULTI_37 values(-9999999999999999999999999999999999999999999999, 1);`) + tk.MustExec(`prepare stmt from 'SELECT SUM(COL1+?), col2 FROM PK_S_MULTI_37 GROUP BY col2';`) + tk.MustExec(`set @a=1;`) + tk.MustQuery(`execute stmt using @a`).Check(testkit.Rows("-9999999999999999999999999999999999999999999998 1")) +} + +func TestIssue28828(t *testing.T) { + store, clean := testkit.CreateMockStore(t) + defer clean() +>>>>>>> eec518648... planner: restrict plan cache for decimal parameter types (#31769) orgEnable := core.PreparedPlanCacheEnabled() defer func() { core.SetPreparedPlanCache(orgEnable)