From 310a2160bef784bbe3910e8af7d0ccdca83901c0 Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Tue, 2 Aug 2022 18:48:50 +0200 Subject: [PATCH 1/5] ddl: Fix for unsigned partitioning expressions --- ddl/partition.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/ddl/partition.go b/ddl/partition.go index 1fcf4ca4f3db0..91e39806533b5 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -660,7 +660,7 @@ func buildRangePartitionDefinitions(ctx sessionctx.Context, defs []*ast.Partitio func checkPartitionValuesIsInt(ctx sessionctx.Context, def *ast.PartitionDefinition, exprs []ast.ExprNode, tbInfo *model.TableInfo) error { tp := types.NewFieldType(mysql.TypeLonglong) - if isColUnsigned(tbInfo.Columns, tbInfo.Partition) { + if isPartExprUnsigned(ctx, tbInfo) { tp.AddFlag(mysql.UnsignedFlag) } for _, exp := range exprs { @@ -812,11 +812,10 @@ func checkRangePartitionValue(ctx sessionctx.Context, tblInfo *model.TableInfo) return nil } - cols := tblInfo.Columns if strings.EqualFold(defs[len(defs)-1].LessThan[0], partitionMaxValue) { defs = defs[:len(defs)-1] } - isUnsigned := isColUnsigned(cols, pi) + isUnsigned := isPartExprUnsigned(ctx, tblInfo) var prevRangeValue interface{} for i := 0; i < len(defs); i++ { if strings.EqualFold(defs[i].LessThan[0], partitionMaxValue) { @@ -879,7 +878,7 @@ func formatListPartitionValue(ctx sessionctx.Context, tblInfo *model.TableInfo) cols := make([]*model.ColumnInfo, 0, len(pi.Columns)) if len(pi.Columns) == 0 { tp := types.NewFieldType(mysql.TypeLonglong) - if isColUnsigned(tblInfo.Columns, tblInfo.Partition) { + if isPartExprUnsigned(ctx, tblInfo) { tp.AddFlag(mysql.UnsignedFlag) } colTps = []*types.FieldType{tp} @@ -1983,13 +1982,13 @@ func (cns columnNameSlice) At(i int) string { return cns[i].Name.L } -// isColUnsigned returns true if the partitioning key column is unsigned. -func isColUnsigned(cols []*model.ColumnInfo, pi *model.PartitionInfo) bool { - for _, col := range cols { - isUnsigned := mysql.HasUnsignedFlag(col.GetFlag()) - if isUnsigned && strings.Contains(strings.ToLower(pi.Expr), col.Name.L) { - return true - } +func isPartExprUnsigned(ctx sessionctx.Context, tbInfo *model.TableInfo) bool { + expr, err := expression.ParseSimpleExprWithTableInfo(ctx, tbInfo.Partition.Expr, tbInfo) + if err != nil { + return false + } + if mysql.HasUnsignedFlag(expr.GetType().GetFlag()) { + return true } return false } From 6b71fd452a7e2556f00fa27cdaaf06fc04d02bf8 Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Tue, 2 Aug 2022 18:56:06 +0200 Subject: [PATCH 2/5] Added test case --- ddl/db_partition_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ddl/db_partition_test.go b/ddl/db_partition_test.go index 5a3bdb7588a8b..55a934a102a84 100644 --- a/ddl/db_partition_test.go +++ b/ddl/db_partition_test.go @@ -333,6 +333,10 @@ partition by range (a) partition p0 values less than (200), partition p1 values less than (300), partition p2 values less than maxvalue)`) + + // Fix https://github.com/pingcap/tidb/issues/35827 + tk.MustExec(`create table t37 (id tinyint unsigned, idpart tinyint, i varchar(255)) partition by range (idpart) (partition p1 values less than (-1));`) + tk.MustGetErrCode(`create table t38 (id tinyint unsigned, idpart tinyint unsigned, i varchar(255)) partition by range (idpart) (partition p1 values less than (-1));`, errno.ErrPartitionConstDomain) } func TestCreateTableWithHashPartition(t *testing.T) { From 0a678162b3b3847431c60372ca9f289568cf279b Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Wed, 3 Aug 2022 16:46:52 +0200 Subject: [PATCH 3/5] Updated with mock ctx for no dependencies of sql_mode etc. --- ddl/partition.go | 12 ++++++++---- table/tables/partition_test.go | 6 ++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/ddl/partition.go b/ddl/partition.go index 91e39806533b5..91a9490576e7b 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -51,6 +51,7 @@ import ( "github.com/pingcap/tidb/util/hack" "github.com/pingcap/tidb/util/logutil" "github.com/pingcap/tidb/util/mathutil" + "github.com/pingcap/tidb/util/mock" "github.com/pingcap/tidb/util/slice" "github.com/pingcap/tidb/util/sqlexec" "github.com/tikv/client-go/v2/tikv" @@ -660,7 +661,7 @@ func buildRangePartitionDefinitions(ctx sessionctx.Context, defs []*ast.Partitio func checkPartitionValuesIsInt(ctx sessionctx.Context, def *ast.PartitionDefinition, exprs []ast.ExprNode, tbInfo *model.TableInfo) error { tp := types.NewFieldType(mysql.TypeLonglong) - if isPartExprUnsigned(ctx, tbInfo) { + if isPartExprUnsigned(tbInfo) { tp.AddFlag(mysql.UnsignedFlag) } for _, exp := range exprs { @@ -815,7 +816,7 @@ func checkRangePartitionValue(ctx sessionctx.Context, tblInfo *model.TableInfo) if strings.EqualFold(defs[len(defs)-1].LessThan[0], partitionMaxValue) { defs = defs[:len(defs)-1] } - isUnsigned := isPartExprUnsigned(ctx, tblInfo) + isUnsigned := isPartExprUnsigned(tblInfo) var prevRangeValue interface{} for i := 0; i < len(defs); i++ { if strings.EqualFold(defs[i].LessThan[0], partitionMaxValue) { @@ -878,7 +879,7 @@ func formatListPartitionValue(ctx sessionctx.Context, tblInfo *model.TableInfo) cols := make([]*model.ColumnInfo, 0, len(pi.Columns)) if len(pi.Columns) == 0 { tp := types.NewFieldType(mysql.TypeLonglong) - if isPartExprUnsigned(ctx, tblInfo) { + if isPartExprUnsigned(tblInfo) { tp.AddFlag(mysql.UnsignedFlag) } colTps = []*types.FieldType{tp} @@ -1982,7 +1983,10 @@ func (cns columnNameSlice) At(i int) string { return cns[i].Name.L } -func isPartExprUnsigned(ctx sessionctx.Context, tbInfo *model.TableInfo) bool { +func isPartExprUnsigned(tbInfo *model.TableInfo) bool { + // We should not rely on any configuration, system or session variables, so use a mock ctx! + // Same as in tables.newPartitionExpr + ctx := mock.NewContext() expr, err := expression.ParseSimpleExprWithTableInfo(ctx, tbInfo.Partition.Expr, tbInfo) if err != nil { return false diff --git a/table/tables/partition_test.go b/table/tables/partition_test.go index 60c32cb41357f..b05cf8f5037bd 100644 --- a/table/tables/partition_test.go +++ b/table/tables/partition_test.go @@ -512,6 +512,12 @@ func TestRangePartitionUnderNoUnsigned(t *testing.T) { tk.MustExec("drop table if exists tu;") defer tk.MustExec("drop table if exists t2;") defer tk.MustExec("drop table if exists tu;") + tk.MustGetErrCode(`CREATE TABLE tu (c1 BIGINT UNSIGNED) PARTITION BY RANGE(c1 - 10) ( + PARTITION p0 VALUES LESS THAN (-5), + PARTITION p1 VALUES LESS THAN (0), + PARTITION p2 VALUES LESS THAN (5), + PARTITION p3 VALUES LESS THAN (10), + PARTITION p4 VALUES LESS THAN (MAXVALUE));`, mysql.ErrPartitionConstDomain) tk.MustExec("SET @@sql_mode='NO_UNSIGNED_SUBTRACTION';") tk.MustExec(`create table t2 (a bigint unsigned) partition by range (a) ( partition p1 values less than (0), From f44bad5284c4dc3f2703261f8e62cb67ea1a5c8c Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Thu, 4 Aug 2022 15:47:47 +0200 Subject: [PATCH 4/5] Added logging if error --- ddl/partition.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ddl/partition.go b/ddl/partition.go index 91a9490576e7b..1184a1298c29a 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -1989,6 +1989,7 @@ func isPartExprUnsigned(tbInfo *model.TableInfo) bool { ctx := mock.NewContext() expr, err := expression.ParseSimpleExprWithTableInfo(ctx, tbInfo.Partition.Expr, tbInfo) if err != nil { + logutil.BgLogger().Error("isPartExpr failed parsing expression!", zap.Error(err)) return false } if mysql.HasUnsignedFlag(expr.GetType().GetFlag()) { From 6eca633f1f0feea19106e3c2f6f3d987c6f2d566 Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Thu, 18 Aug 2022 09:35:21 +0200 Subject: [PATCH 5/5] Replaced isPartExprUnsigned implementation from interval partitioning --- ddl/partition.go | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/ddl/partition.go b/ddl/partition.go index e7b4f7b74f43a..96b735ad51f7f 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -505,20 +505,6 @@ func buildTablePartitionInfo(ctx sessionctx.Context, s *ast.PartitionOptions, tb return nil } -func isPartExprUnsigned(ctx sessionctx.Context, tbInfo *model.TableInfo) bool { - expr, err := expression.ParseSimpleExprWithTableInfo(ctx, tbInfo.Partition.Expr, tbInfo) - if err != nil { - return false - } - pCols := expression.ExtractColumns(expr) - for _, col := range pCols { - if mysql.HasUnsignedFlag(col.GetType().GetFlag()) { - return true - } - } - return false -} - // getPartitionIntervalFromTable checks if a partitioned table matches a generated INTERVAL partitioned scheme // will return nil if error occurs, i.e. not an INTERVAL partitioned table func getPartitionIntervalFromTable(ctx sessionctx.Context, tbInfo *model.TableInfo) *ast.PartitionInterval { @@ -555,7 +541,7 @@ func getPartitionIntervalFromTable(ctx sessionctx.Context, tbInfo *model.TableIn return nil } } else { - if !isPartExprUnsigned(ctx, tbInfo) { + if !isPartExprUnsigned(tbInfo) { minVal = "-9223372036854775808" } } @@ -829,7 +815,7 @@ func generatePartitionDefinitionsFromInterval(ctx sessionctx.Context, partOption if partCol != nil { min = getLowerBoundInt(partCol) } else { - if !isPartExprUnsigned(ctx, tbInfo) { + if !isPartExprUnsigned(tbInfo) { min = math.MinInt64 } }