planner: restrict plan cache for decimal parameter types (#31769)

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 {

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
 }

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

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)