Skip to content

Commit

Permalink
ddl: Ensure correct errors for duplicate constraint names (#48058)
Browse files Browse the repository at this point in the history
close #48040
  • Loading branch information
dveeden authored Oct 31, 2023
1 parent fab7825 commit cc12200
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 10 deletions.
5 changes: 5 additions & 0 deletions errors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 11 additions & 7 deletions pkg/ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
22 changes: 22 additions & 0 deletions pkg/ddl/ddl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'")
}
2 changes: 2 additions & 0 deletions pkg/util/dbterror/ddl_terror.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions tests/integrationtest/r/ddl/db_integration.result
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/integrationtest/r/ddl/multi_schema_change.result
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit cc12200

Please sign in to comment.