From 83e35650f18bb650b5d1eccd85dde340f43eea46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Eeden?= Date: Thu, 7 Sep 2023 19:56:13 +0200 Subject: [PATCH] parser,config: Correct handling of display width deprecation except for zerofill and tinyint(1) (#46651) close pingcap/tidb#46650 --- executor/test/showtest/show_test.go | 12 +++++++++++- parser/types/BUILD.bazel | 2 +- parser/types/field_type.go | 14 ++++++++++++-- parser/types/field_type_test.go | 30 +++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/executor/test/showtest/show_test.go b/executor/test/showtest/show_test.go index b88be4e638..5a65c51bfa 100644 --- a/executor/test/showtest/show_test.go +++ b/executor/test/showtest/show_test.go @@ -1647,12 +1647,22 @@ func TestShowCreateTableWithIntegerDisplayLengthWarnings(t *testing.T) { tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1064 You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use [parser:1681]Integer display width is deprecated and will be removed in a future release.", "Warning 1064 You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use [parser:1681]Integer display width is deprecated and will be removed in a future release.")) tk.MustQuery("show create table t").Check(testkit.Rows("t CREATE TABLE `t` (\n" + - " `a` tinyint DEFAULT NULL,\n" + + " `a` tinyint(1) DEFAULT NULL,\n" + " `b` smallint DEFAULT NULL,\n" + " `c` mediumint DEFAULT NULL,\n" + " `d` int DEFAULT NULL,\n" + " `e` bigint DEFAULT NULL\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(id int primary key, c1 bool, c2 int(10) zerofill)") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1064 You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use [parser:1681]Integer display width is deprecated and will be removed in a future release.")) + tk.MustQuery("show create table t").Check(testkit.Rows("t CREATE TABLE `t` (\n" + + " `id` int NOT NULL,\n" + + " `c1` tinyint(1) DEFAULT NULL,\n" + + " `c2` int(10) unsigned zerofill DEFAULT NULL,\n" + + " PRIMARY KEY (`id`) /*T![clustered_index] CLUSTERED */\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) } func TestShowVar(t *testing.T) { diff --git a/parser/types/BUILD.bazel b/parser/types/BUILD.bazel index fa695269f8..79d488076f 100644 --- a/parser/types/BUILD.bazel +++ b/parser/types/BUILD.bazel @@ -27,7 +27,7 @@ go_test( ], embed = [":types"], flaky = True, - shard_count = 5, + shard_count = 6, deps = [ "//parser", "//parser/ast", diff --git a/parser/types/field_type.go b/parser/types/field_type.go index 464ba38a6c..df14009255 100644 --- a/parser/types/field_type.go +++ b/parser/types/field_type.go @@ -409,10 +409,20 @@ func (ft *FieldType) CompactStr() string { suffix = fmt.Sprintf("(%d,%d)", displayFlen, displayDecimal) case mysql.TypeBit, mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString: suffix = fmt.Sprintf("(%d)", displayFlen) - case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: + case mysql.TypeTiny: + // With display length deprecation active tinyint(1) still has + // a display length to indicate this might have been a BOOL. + // Connectors expect this. + // + // See also: + // https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-19.html + if !TiDBStrictIntegerDisplayWidth || (mysql.HasZerofillFlag(ft.flag) || displayFlen == 1) { + suffix = fmt.Sprintf("(%d)", displayFlen) + } + case mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: // Referring this issue #6688, the integer max display length is deprecated in MySQL 8.0. // Since the length doesn't take any effect in TiDB storage or showing result, we remove it here. - if !TiDBStrictIntegerDisplayWidth { + if !TiDBStrictIntegerDisplayWidth || mysql.HasZerofillFlag(ft.flag) { suffix = fmt.Sprintf("(%d)", displayFlen) } case mysql.TypeYear: diff --git a/parser/types/field_type_test.go b/parser/types/field_type_test.go index 5d458cb38b..28c301f65a 100644 --- a/parser/types/field_type_test.go +++ b/parser/types/field_type_test.go @@ -299,3 +299,33 @@ func TestFieldTypeEqual(t *testing.T) { ft1.SetFlen(23) require.Equal(t, true, ft1.Equal(ft2)) } + +func TestCompactStr(t *testing.T) { + cases := []struct { + t byte // Field Type + flen int // Field Length + flags uint // Field Flags, e.g. ZEROFILL + e1 string // Expected string with TiDBStrictIntegerDisplayWidth disabled + e2 string // Expected string with TiDBStrictIntegerDisplayWidth enabled + }{ + // TINYINT(1) is considered a bool by connectors, this should always display + // the display length. + {mysql.TypeTiny, 1, 0, `tinyint(1)`, `tinyint(1)`}, + {mysql.TypeTiny, 2, 0, `tinyint(2)`, `tinyint`}, + + // If the ZEROFILL flag is set the display length should not be hidden. + {mysql.TypeLong, 10, 0, `int(10)`, `int`}, + {mysql.TypeLong, 10, mysql.ZerofillFlag, `int(10)`, `int(10)`}, + } + for _, cc := range cases { + ft := NewFieldType(cc.t) + ft.SetFlen(cc.flen) + ft.SetFlag(cc.flags) + + TiDBStrictIntegerDisplayWidth = false + require.Equal(t, cc.e1, ft.CompactStr()) + + TiDBStrictIntegerDisplayWidth = true + require.Equal(t, cc.e2, ft.CompactStr()) + } +}