From 2eb3fb1487cba26975a67d2ec2e2e9d1939b9582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Eeden?= Date: Mon, 11 Sep 2023 13:25:51 +0200 Subject: [PATCH] parser,ddl: Add warning for ZEROFILL and improve display width warning (#46759) close pingcap/tidb#46753 --- ddl/db_test.go | 26 +++++++++++------------ ddl/ddl_api.go | 22 ++++++++++++++++++++ errno/errname.go | 2 +- errors.toml | 5 +++++ executor/test/showtest/show_test.go | 32 +++++++++++++++++++++-------- parser/mysql/errname.go | 2 +- parser/parser.go | 6 +----- parser/parser.y | 6 +----- parser/yy_parser.go | 2 -- util/dbterror/ddl_terror.go | 15 ++++++++++++++ 10 files changed, 82 insertions(+), 36 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 6ad8003205..407d2e1734 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -953,7 +953,7 @@ func TestAddIndexFailOnCaseWhenCanExit(t *testing.T) { tk.MustExec("drop table if exists t") } -func TestCreateTableWithIntegerLengthWaring(t *testing.T) { +func TestCreateTableWithIntegerLengthWarning(t *testing.T) { // Inject the strict-integer-display-width variable in parser directly. parsertypes.TiDBStrictIntegerDisplayWidth = true defer func() { parsertypes.TiDBStrictIntegerDisplayWidth = false }() @@ -963,47 +963,47 @@ func TestCreateTableWithIntegerLengthWaring(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a tinyint(1))") - 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 warnings").Check(testkit.Rows()) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a smallint(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a mediumint(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a bigint(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a integer(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") - tk.MustExec("create table t(a int1(1))") - 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.MustExec("create table t(a int1(1))") // Note that int1(1) is tinyint(1) which is boolean-ish + tk.MustQuery("show warnings").Check(testkit.Rows()) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int2(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int3(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int4(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int8(2))") - 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 warnings").Check(testkit.Rows("Warning 1681 Integer display width is deprecated and will be removed in a future release.")) tk.MustExec("drop table if exists t") } diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 95e6c7b9d4..9f502cf05a 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -768,11 +768,33 @@ func buildColumnsAndConstraints( colMap := make(map[string]*table.Column, len(colDefs)) for i, colDef := range colDefs { + if field_types.TiDBStrictIntegerDisplayWidth { + switch colDef.Tp.GetType() { + case mysql.TypeTiny: + // No warning for BOOL-like tinyint(1) + if colDef.Tp.GetFlen() != types.UnspecifiedLength && colDef.Tp.GetFlen() != 1 { + ctx.GetSessionVars().StmtCtx.AppendWarning( + dbterror.ErrWarnDeprecatedIntegerDisplayWidth.GenWithStackByArgs(), + ) + } + case mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: + if colDef.Tp.GetFlen() != types.UnspecifiedLength { + ctx.GetSessionVars().StmtCtx.AppendWarning( + dbterror.ErrWarnDeprecatedIntegerDisplayWidth.GenWithStackByArgs(), + ) + } + } + } col, cts, err := buildColumnAndConstraint(ctx, i, colDef, outPriKeyConstraint, tblCharset, tblCollate) if err != nil { return nil, nil, errors.Trace(err) } col.State = model.StatePublic + if mysql.HasZerofillFlag(col.GetFlag()) { + ctx.GetSessionVars().StmtCtx.AppendWarning( + dbterror.ErrWarnDeprecatedZerofill.GenWithStackByArgs(), + ) + } constraints = append(constraints, cts...) cols = append(cols, col) colMap[colDef.Name.Name.L] = col diff --git a/errno/errname.go b/errno/errname.go index 641e757a64..648310616d 100644 --- a/errno/errname.go +++ b/errno/errname.go @@ -666,7 +666,7 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{ ErrMessageAndStatement: mysql.Message("%s Statement: %s", nil), ErrInsideTransactionPreventsSwitchBinlogFormat: mysql.Message("Cannot modify @@session.binlogFormat inside a transaction", nil), ErrPathLength: mysql.Message("The path specified for %.64s is too long.", nil), - ErrWarnDeprecatedSyntaxNoReplacement: mysql.Message("'%s' is deprecated and will be removed in a future release.", nil), + ErrWarnDeprecatedSyntaxNoReplacement: mysql.Message("%s is deprecated and will be removed in a future release.%s", nil), ErrWrongNativeTableStructure: mysql.Message("Native table '%-.64s'.'%-.64s' has the wrong structure", nil), ErrWrongPerfSchemaUsage: mysql.Message("Invalid performanceSchema usage.", nil), ErrWarnISSkippedTable: mysql.Message("Table '%s'.'%s' was skipped since its definition is being modified by concurrent DDL statement", nil), diff --git a/errors.toml b/errors.toml index 94280d6a41..0ffcb48e05 100644 --- a/errors.toml +++ b/errors.toml @@ -1011,6 +1011,11 @@ error = ''' Statement is unsafe because it uses a system function that may return a different value on the slave ''' +["ddl:1681"] +error = ''' +The ZEROFILL attribute is deprecated and will be removed in a future release. Use the LPAD function to zero-pad numbers, or store the formatted numbers in a CHAR column. +''' + ["ddl:1688"] error = ''' Comment for index '%-.64s' is too long (max = %d) diff --git a/executor/test/showtest/show_test.go b/executor/test/showtest/show_test.go index 5a65c51bfa..607806ef1c 100644 --- a/executor/test/showtest/show_test.go +++ b/executor/test/showtest/show_test.go @@ -1601,7 +1601,9 @@ func TestShowCreateTableWithIntegerDisplayLengthWarnings(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int(2), b varchar(2))") - 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 warnings").Check(testkit.Rows( + "Warning 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` int DEFAULT NULL,\n" + " `b` varchar(2) DEFAULT NULL\n" + @@ -1609,7 +1611,9 @@ func TestShowCreateTableWithIntegerDisplayLengthWarnings(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a bigint(10), b bigint)") - 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 warnings").Check(testkit.Rows( + "Warning 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` bigint DEFAULT NULL,\n" + " `b` bigint DEFAULT NULL\n" + @@ -1618,8 +1622,10 @@ func TestShowCreateTableWithIntegerDisplayLengthWarnings(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a tinyint(5), b tinyint(2), c tinyint)") // Here it will occur 2 warnings. - 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 warnings").Check(testkit.Rows( + "Warning 1681 Integer display width is deprecated and will be removed in a future release.", + "Warning 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" + " `b` tinyint DEFAULT NULL,\n" + @@ -1628,7 +1634,9 @@ func TestShowCreateTableWithIntegerDisplayLengthWarnings(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a smallint(5), b smallint)") - 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 warnings").Check(testkit.Rows( + "Warning 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` smallint DEFAULT NULL,\n" + " `b` smallint DEFAULT NULL\n" + @@ -1636,7 +1644,9 @@ func TestShowCreateTableWithIntegerDisplayLengthWarnings(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a mediumint(5), b mediumint)") - 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 warnings").Check(testkit.Rows( + "Warning 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` mediumint DEFAULT NULL,\n" + " `b` mediumint DEFAULT NULL\n" + @@ -1644,8 +1654,9 @@ func TestShowCreateTableWithIntegerDisplayLengthWarnings(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int1(1), b int2(2), c int3, d int4, e int8)") - 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 warnings").Check(testkit.Rows( + "Warning 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(1) DEFAULT NULL,\n" + " `b` smallint DEFAULT NULL,\n" + @@ -1656,7 +1667,10 @@ func TestShowCreateTableWithIntegerDisplayLengthWarnings(t *testing.T) { 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 warnings").Check(testkit.Rows( + "Warning 1681 Integer display width is deprecated and will be removed in a future release.", + "Warning 1681 The ZEROFILL attribute is deprecated and will be removed in a future release. Use the LPAD function to zero-pad numbers, or store the formatted numbers in a CHAR column.", + )) 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" + diff --git a/parser/mysql/errname.go b/parser/mysql/errname.go index 16b26c8402..6c140adc5f 100644 --- a/parser/mysql/errname.go +++ b/parser/mysql/errname.go @@ -707,7 +707,7 @@ var MySQLErrName = map[uint16]*ErrMessage{ ErrSlaveCantCreateConversion: Message("Can't create conversion table for table '%-.192s.%-.192s'", nil), ErrInsideTransactionPreventsSwitchBinlogFormat: Message("Cannot modify @@session.binlogFormat inside a transaction", nil), ErrPathLength: Message("The path specified for %.64s is too long.", nil), - ErrWarnDeprecatedSyntaxNoReplacement: Message("'%s' is deprecated and will be removed in a future release.", nil), + ErrWarnDeprecatedSyntaxNoReplacement: Message("%s is deprecated and will be removed in a future release.%s", nil), ErrWrongNativeTableStructure: Message("Native table '%-.64s'.'%-.64s' has the wrong structure", nil), ErrWrongPerfSchemaUsage: Message("Invalid performanceSchema usage.", nil), ErrWarnISSkippedTable: Message("Table '%s'.'%s' was skipped since its definition is being modified by concurrent DDL statement", nil), diff --git a/parser/parser.go b/parser/parser.go index f45481d47a..da0ba005ba 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -15925,7 +15925,7 @@ yynewstate: } case 531: { - yylex.AppendError(ErrWarnDeprecatedSyntaxNoReplacement.FastGenByArgs("DROP STATS ... PARTITION ...")) + yylex.AppendError(ErrWarnDeprecatedSyntaxNoReplacement.FastGenByArgs("'DROP STATS ... PARTITION ...'", "")) parser.lastErrorAsWarn() parser.yyVAL.statement = &ast.DropStatsStmt{ Tables: []*ast.TableName{yyS[yypt-2].item.(*ast.TableName)}, @@ -21729,10 +21729,6 @@ yynewstate: // TODO: check flen 0 tp := types.NewFieldType(yyS[yypt-2].item.(byte)) tp.SetFlen(yyS[yypt-1].item.(int)) - if yyS[yypt-1].item.(int) != types.UnspecifiedLength && types.TiDBStrictIntegerDisplayWidth { - yylex.AppendError(ErrWarnDeprecatedIntegerDisplayWidth) - parser.lastErrorAsWarn() - } for _, o := range yyS[yypt-0].item.([]*ast.TypeOpt) { if o.IsUnsigned { tp.AddFlag(mysql.UnsignedFlag) diff --git a/parser/parser.y b/parser/parser.y index 29fe8eee29..1adb07cce2 100644 --- a/parser/parser.y +++ b/parser/parser.y @@ -5178,7 +5178,7 @@ DropStatsStmt: } | "DROP" "STATS" TableName "PARTITION" PartitionNameList { - yylex.AppendError(ErrWarnDeprecatedSyntaxNoReplacement.FastGenByArgs("DROP STATS ... PARTITION ...")) + yylex.AppendError(ErrWarnDeprecatedSyntaxNoReplacement.FastGenByArgs("'DROP STATS ... PARTITION ...'","")) parser.lastErrorAsWarn() $$ = &ast.DropStatsStmt{ Tables: []*ast.TableName{$3.(*ast.TableName)}, @@ -12461,10 +12461,6 @@ NumericType: // TODO: check flen 0 tp := types.NewFieldType($1.(byte)) tp.SetFlen($2.(int)) - if $2.(int) != types.UnspecifiedLength && types.TiDBStrictIntegerDisplayWidth { - yylex.AppendError(ErrWarnDeprecatedIntegerDisplayWidth) - parser.lastErrorAsWarn() - } for _, o := range $3.([]*ast.TypeOpt) { if o.IsUnsigned { tp.AddFlag(mysql.UnsignedFlag) diff --git a/parser/yy_parser.go b/parser/yy_parser.go index 48409d8108..a7c47ea1fe 100644 --- a/parser/yy_parser.go +++ b/parser/yy_parser.go @@ -56,8 +56,6 @@ var ( ErrWarnDeprecatedSyntax = terror.ClassParser.NewStd(mysql.ErrWarnDeprecatedSyntax) // ErrWarnDeprecatedSyntaxNoReplacement return when the syntax was deprecated and there is no replacement. ErrWarnDeprecatedSyntaxNoReplacement = terror.ClassParser.NewStd(mysql.ErrWarnDeprecatedSyntaxNoReplacement) - // ErrWarnDeprecatedIntegerDisplayWidth share the same code 1681, and it will be returned when length is specified in integer. - ErrWarnDeprecatedIntegerDisplayWidth = terror.ClassParser.NewStdErr(mysql.ErrWarnDeprecatedSyntaxNoReplacement, mysql.Message("Integer display width is deprecated and will be removed in a future release.", nil)) // ErrWrongUsage returns for incorrect usages. ErrWrongUsage = terror.ClassParser.NewStd(mysql.ErrWrongUsage) // SpecFieldPattern special result field pattern diff --git a/util/dbterror/ddl_terror.go b/util/dbterror/ddl_terror.go index d3b35a1fa2..80d239c86b 100644 --- a/util/dbterror/ddl_terror.go +++ b/util/dbterror/ddl_terror.go @@ -469,6 +469,21 @@ var ( ErrCheckConstraintUsingFKReferActionColumn = ClassDDL.NewStd(mysql.ErrCheckConstraintClauseUsingFKReferActionColumn) // ErrNonBooleanExprForCheckConstraint is returned for non bool expression. ErrNonBooleanExprForCheckConstraint = ClassDDL.NewStd(mysql.ErrNonBooleanExprForCheckConstraint) + // ErrWarnDeprecatedIntegerDisplayWidth share the same code 1681, and it will be returned when length is specified in integer. + ErrWarnDeprecatedIntegerDisplayWidth = ClassDDL.NewStdErr( + mysql.ErrWarnDeprecatedSyntaxNoReplacement, + parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrWarnDeprecatedSyntaxNoReplacement].Raw, + "Integer display width", "", + ), nil), + ) + // ErrWarnDeprecatedZerofill is for when the deprectated zerofill attribute is used + ErrWarnDeprecatedZerofill = ClassDDL.NewStdErr( + mysql.ErrWarnDeprecatedSyntaxNoReplacement, + parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrWarnDeprecatedSyntaxNoReplacement].Raw, + "The ZEROFILL attribute", + " Use the LPAD function to zero-pad numbers, or store the formatted numbers in a CHAR column.", + ), nil), + ) ) // ReorgRetryableErrCodes is the error codes that are retryable for reorganization.