ttl: forbid creating/altering a table with TTL options when pk contai…

…ns float/double column (#40487)

* ttl: forbid create/alter a table with TTL options when pk contains float/double column

* format

* update

* update

* update

Co-authored-by: Ti Chi Robot <[email protected]>

ddl/ttl.go

@@ -26,6 +26,7 @@ import (
 	"github.com/pingcap/tidb/parser/duration"
 	"github.com/pingcap/tidb/parser/format"
 	"github.com/pingcap/tidb/parser/model"
+	"github.com/pingcap/tidb/parser/mysql"
 	"github.com/pingcap/tidb/sessionctx"
 	"github.com/pingcap/tidb/sessiontxn"
 	"github.com/pingcap/tidb/types"
@@ -143,6 +144,10 @@ func checkTTLTableSuitable(ctx sessionctx.Context, schema model.CIStr, tblInfo *
 		return dbterror.ErrTempTableNotAllowedWithTTL
 	}
 
+	if err := checkPrimaryKeyForTTLTable(tblInfo); err != nil {
+		return err
+	}
+
 	// checks even when the foreign key check is not enabled, to keep safe
 	is := sessiontxn.GetTxnManager(ctx).GetTxnInfoSchema()
 	if referredFK := checkTableHasForeignKeyReferred(is, schema.L, tblInfo.Name.L, nil, true); referredFK != nil {
@@ -162,6 +167,31 @@ func checkDropColumnWithTTLConfig(tblInfo *model.TableInfo, colName string) erro
 	return nil
 }
 
+// We should forbid creating a TTL table with clustered primary key that contains a column with type float/double.
+// This is because currently we are using SQL to delete expired rows and when the primary key contains float/double column,
+// it is hard to use condition `WHERE PK in (...)` to delete specified rows because some precision will be lost when comparing.
+func checkPrimaryKeyForTTLTable(tblInfo *model.TableInfo) error {
+	if !tblInfo.IsCommonHandle {
+		// only check the primary keys when it is common handle
+		return nil
+	}
+
+	pk := tblInfo.GetPrimaryKey()
+	if pk == nil {
+		return nil
+	}
+
+	for _, colDef := range pk.Columns {
+		col := tblInfo.Columns[colDef.Offset]
+		switch col.GetType() {
+		case mysql.TypeFloat, mysql.TypeDouble:
+			return dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL
+		}
+	}
+
+	return nil
+}
+
 // getTTLInfoInOptions returns the aggregated ttlInfo, the ttlEnable, or an error.
 // if TTL, TTL_ENABLE or TTL_JOB_INTERVAL is not set in the config, the corresponding return value will be nil.
 // if both of TTL and TTL_ENABLE are set, the `ttlInfo.Enable` will be equal with `ttlEnable`.

errno/errcode.go

@@ -1043,6 +1043,7 @@ const (
 	ErrSetTTLOptionForNonTTLTable          = 8150
 	ErrTempTableNotAllowedWithTTL          = 8151
 	ErrUnsupportedTTLReferencedByFK        = 8152
+	ErrUnsupportedPrimaryKeyTypeWithTTL    = 8153
 
 	// Error codes used by TiDB ddl package
 	ErrUnsupportedDDLOperation            = 8200

errors.toml

@@ -1236,6 +1236,11 @@ error = '''
 Set TTL for a table referenced by foreign key is not allowed
 '''
 
+["ddl:8153"]
+error = '''
+Unsupported clustered primary key type FLOAT/DOUBLE for TTL
+'''
+
 ["ddl:8200"]
 error = '''
 Unsupported shard_row_id_bits for table with primary key as row id

executor/ddl_test.go

@@ -1646,3 +1646,40 @@ func TestDisableTTLForFKParentTable(t *testing.T) {
 	tk.MustGetDBError("ALTER TABLE t_1 ADD FOREIGN KEY fk_t_id(t_id) references t(id)", dbterror.ErrUnsupportedTTLReferencedByFK)
 	tk.MustExec("drop table t,t_1")
 }
+
+func TestCheckPrimaryKeyForTTLTable(t *testing.T) {
+	store := testkit.CreateMockStore(t)
+	tk := testkit.NewTestKit(t, store)
+	tk.MustExec("use test")
+
+	// create table should fail when pk contains double/float
+	tk.MustGetDBError("create table t1(id float primary key, t timestamp) TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+	tk.MustGetDBError("create table t1(id float(10,2) primary key, t timestamp) TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+	tk.MustGetDBError("create table t1(id double primary key, t timestamp) TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+	tk.MustGetDBError("create table t1(id float(10,2) primary key, t timestamp) TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+	tk.MustGetDBError("create table t1(id1 int, id2 float, t timestamp, primary key(id1, id2)) TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+	tk.MustGetDBError("create table t1(id1 int, id2 double, t timestamp, primary key(id1, id2)) TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+
+	// alter table should fail when pk contains double/float
+	tk.MustExec("create table t1(id float primary key, t timestamp)")
+	tk.MustExec("create table t2(id double primary key, t timestamp)")
+	tk.MustExec("create table t3(id1 int, id2 float, primary key(id1, id2), t timestamp)")
+	tk.MustExec("create table t4(id1 int, id2 double, primary key(id1, id2), t timestamp)")
+	tk.MustGetDBError("alter table t1 TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+	tk.MustGetDBError("alter table t2 TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+	tk.MustGetDBError("alter table t3 TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+	tk.MustGetDBError("alter table t4 TTL=`t`+INTERVAL 1 DAY", dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL)
+
+	// create table should not fail when the pk is not clustered
+	tk.MustExec("create table t11(id float primary key nonclustered, t timestamp) TTL=`t`+INTERVAL 1 DAY")
+	tk.MustExec("create table t12(id double primary key nonclustered, t timestamp) TTL=`t`+INTERVAL 1 DAY")
+	tk.MustExec("create table t13(id1 int, id2 float, t timestamp, primary key(id1, id2) nonclustered) TTL=`t`+INTERVAL 1 DAY")
+
+	// alter table should not fail when the pk is not clustered
+	tk.MustExec("create table t21(id float primary key nonclustered, t timestamp)")
+	tk.MustExec("create table t22(id double primary key nonclustered, t timestamp)")
+	tk.MustExec("create table t23(id1 int, id2 float, t timestamp, primary key(id1, id2) nonclustered)")
+	tk.MustExec("alter table t21 TTL=`t`+INTERVAL 1 DAY")
+	tk.MustExec("alter table t22 TTL=`t`+INTERVAL 1 DAY")
+	tk.MustExec("alter table t23 TTL=`t`+INTERVAL 1 DAY")
+}

ttl/cache/split_test.go

@@ -451,7 +451,6 @@ func TestNoTTLSplitSupportTables(t *testing.T) {
 	tbls := []*cache.PhysicalTable{
 		createTTLTable(t, tk, "t1", "char(32)  CHARACTER SET UTF8MB4"),
 		createTTLTable(t, tk, "t2", "varchar(32) CHARACTER SET UTF8MB4"),
-		createTTLTable(t, tk, "t3", "double"),
 		createTTLTable(t, tk, "t4", "decimal(32, 2)"),
 		create2PKTTLTable(t, tk, "t5", "char(32)  CHARACTER SET UTF8MB4"),
 	}

ttl/sqlbuilder/BUILD.bazel

@@ -31,10 +31,12 @@ go_test(
         "//parser/ast",
         "//parser/model",
         "//parser/mysql",
+        "//parser/terror",
         "//testkit",
         "//testkit/testsetup",
         "//ttl/cache",
         "//types",
+        "//util/dbterror",
         "//util/sqlexec",
         "@com_github_stretchr_testify//require",
         "@org_uber_go_goleak//:goleak",

ttl/sqlbuilder/sql_test.go

@@ -26,10 +26,12 @@ import (
 	"github.com/pingcap/tidb/parser/ast"
 	"github.com/pingcap/tidb/parser/model"
 	"github.com/pingcap/tidb/parser/mysql"
+	"github.com/pingcap/tidb/parser/terror"
 	"github.com/pingcap/tidb/testkit"
 	"github.com/pingcap/tidb/ttl/cache"
 	"github.com/pingcap/tidb/ttl/sqlbuilder"
 	"github.com/pingcap/tidb/types"
+	"github.com/pingcap/tidb/util/dbterror"
 	"github.com/pingcap/tidb/util/sqlexec"
 	"github.com/stretchr/testify/require"
 )
@@ -162,40 +164,56 @@ func TestFormatSQLDatum(t *testing.T) {
 	// invalid pk types contains the types that should not exist in primary keys of a TTL table.
 	// We do not need to check sqlbuilder.FormatSQLDatum for these types
 	invalidPKTypes := []struct {
-		types  []string
-		errMsg string
+		types []string
+		err   *terror.Error
 	}{
 		{
-			types:  []string{"json"},
-			errMsg: "[ddl:3152]JSON column 'pk0' cannot be used in key specification.",
+			types: []string{"json"},
+			err:   dbterror.ErrJSONUsedAsKey,
 		},
 		{
-			types:  []string{"blob"},
-			errMsg: "[ddl:1170]BLOB/TEXT column 'pk0' used in key specification without a key length",
+			types: []string{"blob"},
+			err:   dbterror.ErrBlobKeyWithoutLength,
 		},
 		{
-			types:  []string{"blob(8)"},
-			errMsg: "[ddl:1170]BLOB/TEXT column 'pk0' used in key specification without a key length",
+			types: []string{"blob(8)"},
+			err:   dbterror.ErrBlobKeyWithoutLength,
 		},
 		{
-			types:  []string{"text"},
-			errMsg: "[ddl:1170]BLOB/TEXT column 'pk0' used in key specification without a key length",
+			types: []string{"text"},
+			err:   dbterror.ErrBlobKeyWithoutLength,
 		},
 		{
-			types:  []string{"text(8)"},
-			errMsg: "[ddl:1170]BLOB/TEXT column 'pk0' used in key specification without a key length",
+			types: []string{"text(8)"},
+			err:   dbterror.ErrBlobKeyWithoutLength,
 		},
 		{
-			types:  []string{"int", "json"},
-			errMsg: "[ddl:3152]JSON column 'pk1' cannot be used in key specification.",
+			types: []string{"int", "json"},
+			err:   dbterror.ErrJSONUsedAsKey,
 		},
 		{
-			types:  []string{"int", "blob"},
-			errMsg: "[ddl:1170]BLOB/TEXT column 'pk1' used in key specification without a key length",
+			types: []string{"int", "blob"},
+			err:   dbterror.ErrBlobKeyWithoutLength,
 		},
 		{
-			types:  []string{"int", "text"},
-			errMsg: "[ddl:1170]BLOB/TEXT column 'pk1' used in key specification without a key length",
+			types: []string{"int", "text"},
+			err:   dbterror.ErrBlobKeyWithoutLength,
+		},
+		{
+			types: []string{"float"},
+			err:   dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL,
+		},
+		{
+			types: []string{"double"},
+			err:   dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL,
+		},
+		{
+			types: []string{"int", "float"},
+			err:   dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL,
+		},
+		{
+			types: []string{"int", "double"},
+			err:   dbterror.ErrUnsupportedPrimaryKeyTypeWithTTL,
 		},
 	}
 
@@ -321,8 +339,7 @@ func TestFormatSQLDatum(t *testing.T) {
 		sb.WriteString("primary key (")
 		sb.WriteString(strings.Join(cols, ", "))
 		sb.WriteString(")) TTL=`t` + INTERVAL 1 DAY")
-		err := tk.ExecToErr(sb.String())
-		require.Equal(t, c.errMsg, err.Error(), sb.String())
+		tk.MustGetDBError(sb.String(), c.err)
 	}
 
 	// create a table with n columns

util/dbterror/ddl_terror.go

@@ -431,6 +431,8 @@ var (
 	ErrTempTableNotAllowedWithTTL = ClassDDL.NewStd(mysql.ErrTempTableNotAllowedWithTTL)
 	// ErrUnsupportedTTLReferencedByFK returns when the TTL config is set for a table referenced by foreign key
 	ErrUnsupportedTTLReferencedByFK = ClassDDL.NewStd(mysql.ErrUnsupportedTTLReferencedByFK)
+	// ErrUnsupportedPrimaryKeyTypeWithTTL returns when create or alter a table with TTL options but the primary key is not supported
+	ErrUnsupportedPrimaryKeyTypeWithTTL = ClassDDL.NewStd(mysql.ErrUnsupportedPrimaryKeyTypeWithTTL)
 
 	// ErrNotSupportedYet returns when tidb does not support this feature.
 	ErrNotSupportedYet = ClassDDL.NewStd(mysql.ErrNotSupportedYet)