From cc122007959ca32625e7e38795a1bd2e4ffd8e64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Eeden?= Date: Tue, 31 Oct 2023 06:03:37 +0100 Subject: [PATCH] ddl: Ensure correct errors for duplicate constraint names (#48058) close pingcap/tidb#48040 --- errors.toml | 5 +++++ pkg/ddl/ddl_api.go | 18 +++++++++------ pkg/ddl/ddl_test.go | 22 +++++++++++++++++++ pkg/util/dbterror/ddl_terror.go | 2 ++ .../r/ddl/db_integration.result | 4 ++-- .../r/ddl/multi_schema_change.result | 2 +- 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/errors.toml b/errors.toml index 62c4f3927eea2..35eef4500f3b2 100644 --- a/errors.toml +++ b/errors.toml @@ -1251,6 +1251,11 @@ error = ''' Check constraint '%s' refers to non-existing column '%s'. ''' +["ddl:3822"] +error = ''' +Duplicate check constraint name '%s'. +''' + ["ddl:3823"] error = ''' Column '%s' cannot be used in a check constraint '%s': needed in a foreign key constraint referential action. diff --git a/pkg/ddl/ddl_api.go b/pkg/ddl/ddl_api.go index 4cd6ce1503ac6..d80aec23dce38 100644 --- a/pkg/ddl/ddl_api.go +++ b/pkg/ddl/ddl_api.go @@ -1760,16 +1760,20 @@ func checkColumnAttributes(colName string, tp *types.FieldType) error { return nil } -func checkDuplicateConstraint(namesMap map[string]bool, name string, foreign bool) error { +func checkDuplicateConstraint(namesMap map[string]bool, name string, constraintType ast.ConstraintType) error { if name == "" { return nil } nameLower := strings.ToLower(name) if namesMap[nameLower] { - if foreign { + switch constraintType { + case ast.ConstraintForeignKey: return dbterror.ErrFkDupName.GenWithStackByArgs(name) + case ast.ConstraintCheck: + return dbterror.ErrCheckConstraintDupName.GenWithStackByArgs(name) + default: + return dbterror.ErrDupKeyName.GenWithStackByArgs(name) } - return dbterror.ErrDupKeyName.GenWithStack("duplicate key name %s", name) } namesMap[nameLower] = true return nil @@ -1829,12 +1833,12 @@ func checkConstraintNames(tableName model.CIStr, constraints []*ast.Constraint) // Check not empty constraint name whether is duplicated. for _, constr := range constraints { if constr.Tp == ast.ConstraintForeignKey { - err := checkDuplicateConstraint(fkNames, constr.Name, true) + err := checkDuplicateConstraint(fkNames, constr.Name, constr.Tp) if err != nil { return errors.Trace(err) } } else { - err := checkDuplicateConstraint(constrNames, constr.Name, false) + err := checkDuplicateConstraint(constrNames, constr.Name, constr.Tp) if err != nil { return errors.Trace(err) } @@ -7248,11 +7252,11 @@ func (d *ddl) createIndex(ctx sessionctx.Context, ti ast.Ident, keyType ast.Inde if indexInfo := t.Meta().FindIndexByName(indexName.L); indexInfo != nil { if indexInfo.State != model.StatePublic { // NOTE: explicit error message. See issue #18363. - err = dbterror.ErrDupKeyName.GenWithStack("index already exist %s; "+ + err = dbterror.ErrDupKeyName.GenWithStack("Duplicate key name '%s'; "+ "a background job is trying to add the same index, "+ "please check by `ADMIN SHOW DDL JOBS`", indexName) } else { - err = dbterror.ErrDupKeyName.GenWithStack("index already exist %s", indexName) + err = dbterror.ErrDupKeyName.GenWithStackByArgs(indexName) } if ifNotExists { ctx.GetSessionVars().StmtCtx.AppendNote(err) diff --git a/pkg/ddl/ddl_test.go b/pkg/ddl/ddl_test.go index 7b8df8620df02..a59c58833fc88 100644 --- a/pkg/ddl/ddl_test.go +++ b/pkg/ddl/ddl_test.go @@ -285,3 +285,25 @@ func TestError(t *testing.T) { require.Equal(t, uint16(err.Code()), code) } } + +func TestCheckDuplicateConstraint(t *testing.T) { + constrNames := map[string]bool{} + + // Foreign Key + err := checkDuplicateConstraint(constrNames, "f1", ast.ConstraintForeignKey) + require.NoError(t, err) + err = checkDuplicateConstraint(constrNames, "f1", ast.ConstraintForeignKey) + require.EqualError(t, err, "[ddl:1826]Duplicate foreign key constraint name 'f1'") + + // Check constraint + err = checkDuplicateConstraint(constrNames, "c1", ast.ConstraintCheck) + require.NoError(t, err) + err = checkDuplicateConstraint(constrNames, "c1", ast.ConstraintCheck) + require.EqualError(t, err, "[ddl:3822]Duplicate check constraint name 'c1'.") + + // Unique contraints etc + err = checkDuplicateConstraint(constrNames, "u1", ast.ConstraintUniq) + require.NoError(t, err) + err = checkDuplicateConstraint(constrNames, "u1", ast.ConstraintUniq) + require.EqualError(t, err, "[ddl:1061]Duplicate key name 'u1'") +} diff --git a/pkg/util/dbterror/ddl_terror.go b/pkg/util/dbterror/ddl_terror.go index de98d1b9912b1..67861cfec2d7b 100644 --- a/pkg/util/dbterror/ddl_terror.go +++ b/pkg/util/dbterror/ddl_terror.go @@ -484,6 +484,8 @@ var ( " Use the LPAD function to zero-pad numbers, or store the formatted numbers in a CHAR column.", ), nil), ) + // ErrCheckConstraintDupName is for duplicate check constraint names + ErrCheckConstraintDupName = ClassDDL.NewStd(mysql.ErrCheckConstraintDupName) ) // ReorgRetryableErrCodes is the error codes that are retryable for reorganization. diff --git a/tests/integrationtest/r/ddl/db_integration.result b/tests/integrationtest/r/ddl/db_integration.result index 515bb8774d5a6..cf6f13e14cdc3 100644 --- a/tests/integrationtest/r/ddl/db_integration.result +++ b/tests/integrationtest/r/ddl/db_integration.result @@ -139,7 +139,7 @@ Error 1166 (42000): Incorrect column name '_tidb_rowid' create table aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(a int); Error 1059 (42000): Identifier name 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' is too long create table test_error_code1 (c1 int, c2 int, key aa (c1, c2), key aa (c1)); -Error 1061 (42000): duplicate key name aa +Error 1061 (42000): Duplicate key name 'aa' create table test_error_code1 (c1 int, c2 int, c3 int, key(c_not_exist)); Error 1072 (42000): column does not exist: c_not_exist create table test_error_code1 (c1 int, c2 int, c3 int, primary key(c_not_exist)); @@ -240,7 +240,7 @@ alter table test_error_code_succ add index idx (c_not_exist); Error 1072 (42000): column does not exist: c_not_exist alter table test_error_code_succ add index idx (c1); alter table test_error_code_succ add index idx (c1); -Error 1061 (42000): index already exist idx +Error 1061 (42000): Duplicate key name 'idx' alter table test_error_code_succ drop index idx_not_exist; Error 1091 (42000): index idx_not_exist doesn't exist alter table test_error_code_succ drop column c3; diff --git a/tests/integrationtest/r/ddl/multi_schema_change.result b/tests/integrationtest/r/ddl/multi_schema_change.result index d76c1638e6fc4..c1668d475e442 100644 --- a/tests/integrationtest/r/ddl/multi_schema_change.result +++ b/tests/integrationtest/r/ddl/multi_schema_change.result @@ -217,7 +217,7 @@ Error 1176 (42000): Key 't' doesn't exist in table 't' drop table if exists t; create table t (a int, b int, c int, index t(a)); alter table t drop index t, add index t(b); -Error 1061 (42000): index already exist t +Error 1061 (42000): Duplicate key name 't' drop table if exists t; create table t (a int, b int, c int, index t(a)); alter table t add index t1(b), drop index t1;