Skip to content

Commit

Permalink
ddl: Refine the error message to compatible with MySQL when drop a pa…
Browse files Browse the repository at this point in the history
…rtition table partition key column (#38740)

close #38739
  • Loading branch information
jiyfhust authored Dec 27, 2022
1 parent a94cde3 commit 83d275c
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 4 deletions.
3 changes: 1 addition & 2 deletions ddl/column_modify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,7 @@ func TestDropColumn(t *testing.T) {
tk.MustExec("drop table if exists t1")
tk.MustExec("create table t1 (a int,b int) partition by hash(a) partitions 4;")
err := tk.ExecToErr("alter table t1 drop column a")
// TODO: refine the error message to compatible with MySQL
require.EqualError(t, err, "[planner:1054]Unknown column 'a' in 'expression'")
require.EqualError(t, err, "[ddl:3885]Column 'a' has a partitioning function dependency and cannot be dropped or renamed")
}

func TestChangeColumn(t *testing.T) {
Expand Down
32 changes: 32 additions & 0 deletions ddl/db_partition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4549,3 +4549,35 @@ func TestAlterModifyColumnOnPartitionedTableRename(t *testing.T) {
tk.MustExec(`create table t (a int, b char) partition by hash (a) partitions 3`)
tk.MustContainErrMsg(`alter table t change a c int`, "[ddl:8200]Unsupported modify column: Column 'a' has a partitioning function dependency and cannot be renamed")
}

func TestDropPartitionKeyColumn(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("create database DropPartitionKeyColumn")
defer tk.MustExec("drop database DropPartitionKeyColumn")
tk.MustExec("use DropPartitionKeyColumn")

tk.MustExec("create table t1 (a tinyint, b char) partition by range (a) ( partition p0 values less than (10) )")
err := tk.ExecToErr("alter table t1 drop column a")
require.Error(t, err)
require.Equal(t, "[ddl:3885]Column 'a' has a partitioning function dependency and cannot be dropped or renamed", err.Error())
tk.MustExec("alter table t1 drop column b")

tk.MustExec("create table t2 (a tinyint, b char) partition by range (a-1) ( partition p0 values less than (10) )")
err = tk.ExecToErr("alter table t2 drop column a")
require.Error(t, err)
require.Equal(t, "[ddl:3885]Column 'a' has a partitioning function dependency and cannot be dropped or renamed", err.Error())
tk.MustExec("alter table t2 drop column b")

tk.MustExec("create table t3 (a tinyint, b char) partition by hash(a) partitions 4;")
err = tk.ExecToErr("alter table t3 drop column a")
require.Error(t, err)
require.Equal(t, "[ddl:3885]Column 'a' has a partitioning function dependency and cannot be dropped or renamed", err.Error())
tk.MustExec("alter table t3 drop column b")

tk.MustExec("create table t4 (a char, b char) partition by list columns (a) ( partition p0 values in ('0'), partition p1 values in ('a'), partition p2 values in ('b'));")
err = tk.ExecToErr("alter table t4 drop column a")
require.Error(t, err)
require.Equal(t, "[ddl:3885]Column 'a' has a partitioning function dependency and cannot be dropped or renamed", err.Error())
tk.MustExec("alter table t4 drop column b")
}
3 changes: 1 addition & 2 deletions ddl/db_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -871,8 +871,7 @@ func TestDDLWithInvalidTableInfo(t *testing.T) {

tk.MustExec("create table t (a bigint, b int, c int generated always as (b+1)) partition by hash(a) partitions 4;")
// Test drop partition column.
// TODO: refine the error message to compatible with MySQL
tk.MustGetErrMsg("alter table t drop column a;", "[planner:1054]Unknown column 'a' in 'expression'")
tk.MustGetErrMsg("alter table t drop column a;", "[ddl:3885]Column 'a' has a partitioning function dependency and cannot be dropped or renamed")
// Test modify column with invalid expression.
tk.MustGetErrMsg("alter table t modify column c int GENERATED ALWAYS AS ((case when (a = 0) then 0when (a > 0) then (b / a) end));", "[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 97 near \"then (b / a) end));\" ")
// Test add column with invalid expression.
Expand Down
21 changes: 21 additions & 0 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4306,6 +4306,9 @@ func checkIsDroppableColumn(ctx sessionctx.Context, is infoschema.InfoSchema, sc
if err = isDroppableColumn(tblInfo, colName); err != nil {
return false, errors.Trace(err)
}
if err = checkDropColumnWithPartitionConstraint(t, colName); err != nil {
return false, errors.Trace(err)
}
// Check the column with foreign key.
err = checkDropColumnWithForeignKeyConstraint(is, schema.Name.L, tblInfo, colName.L)
if err != nil {
Expand All @@ -4326,6 +4329,24 @@ func checkIsDroppableColumn(ctx sessionctx.Context, is infoschema.InfoSchema, sc
return true, nil
}

// checkDropColumnWithPartitionConstraint is used to check the partition constraint of the drop column.
func checkDropColumnWithPartitionConstraint(t table.Table, colName model.CIStr) error {
if t.Meta().Partition == nil {
return nil
}
pt, ok := t.(table.PartitionedTable)
if !ok {
// Should never happen!
return errors.Trace(dbterror.ErrDependentByPartitionFunctional.GenWithStackByArgs(colName.L))
}
for _, name := range pt.GetPartitionColumnNames() {
if strings.EqualFold(name.L, colName.L) {
return errors.Trace(dbterror.ErrDependentByPartitionFunctional.GenWithStackByArgs(colName.L))
}
}
return nil
}

func checkVisibleColumnCnt(t table.Table, addCnt, dropCnt int) error {
tblInfo := t.Meta()
visibleColumCnt := 0
Expand Down
1 change: 1 addition & 0 deletions errno/errcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,7 @@ const (
ErrFunctionalIndexRowValueIsNotAllowed = 3800
ErrDependentByFunctionalIndex = 3837
ErrCannotConvertString = 3854
ErrDependentByPartitionFunctional = 3885
ErrInvalidJSONValueForFuncIndex = 3903
ErrJSONValueOutOfRangeForFuncIndex = 3904
ErrFunctionalIndexDataIsTooLong = 3907
Expand Down
1 change: 1 addition & 0 deletions errno/errname.go
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,7 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{
ErrFKIncompatibleColumns: mysql.Message("Referencing column '%s' and referenced column '%s' in foreign key constraint '%s' are incompatible.", nil),
ErrFunctionalIndexRowValueIsNotAllowed: mysql.Message("Expression of expression index '%s' cannot refer to a row value", nil),
ErrDependentByFunctionalIndex: mysql.Message("Column '%s' has an expression index dependency and cannot be dropped or renamed", nil),
ErrDependentByPartitionFunctional: mysql.Message("Column '%s' has a partitioning function dependency and cannot be dropped or renamed", nil),
ErrCannotConvertString: mysql.Message("Cannot convert string '%.64s' from %s to %s", nil),
ErrInvalidJSONValueForFuncIndex: mysql.Message("Invalid JSON value for CAST for expression index '%s'", nil),
ErrJSONValueOutOfRangeForFuncIndex: mysql.Message("Out of range JSON value for CAST for expression index '%s'", nil),
Expand Down
5 changes: 5 additions & 0 deletions errors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,11 @@ error = '''
Column '%s' has an expression index dependency and cannot be dropped or renamed
'''

["ddl:3885"]
error = '''
Column '%s' has a partitioning function dependency and cannot be dropped or renamed
'''

["ddl:4135"]
error = '''
Sequence '%-.64s.%-.64s' has run out
Expand Down
1 change: 1 addition & 0 deletions parser/mysql/errcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,7 @@ const (
ErrFKIncompatibleColumns = 3780
ErrFunctionalIndexRowValueIsNotAllowed = 3800
ErrDependentByFunctionalIndex = 3837
ErrDependentByPartitionFunctional = 3885
ErrInvalidJsonValueForFuncIndex = 3903 //nolint: revive
ErrJsonValueOutOfRangeForFuncIndex = 3904 //nolint: revive
ErrFunctionalIndexDataIsTooLong = 3907
Expand Down
1 change: 1 addition & 0 deletions parser/mysql/errname.go
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,7 @@ var MySQLErrName = map[uint16]*ErrMessage{
ErrJsonValueOutOfRangeForFuncIndex: Message("Out of range JSON value for CAST for functional index '%s'", nil),
ErrFunctionalIndexDataIsTooLong: Message("Data too long for functional index '%s'", nil),
ErrFunctionalIndexNotApplicable: Message("Cannot use functional index '%s' due to type or collation conversion", nil),
ErrDependentByPartitionFunctional: Message("Column '%s' has a partitioning function dependency and cannot be dropped or renamed", nil),

// MariaDB errors.
ErrOnlyOneDefaultPartionAllowed: Message("Only one DEFAULT partition allowed", nil),
Expand Down
2 changes: 2 additions & 0 deletions util/dbterror/ddl_terror.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,8 @@ var (
ErrDependentByFunctionalIndex = ClassDDL.NewStd(mysql.ErrDependentByFunctionalIndex)
// ErrFunctionalIndexOnBlob when the expression of expression index returns blob or text.
ErrFunctionalIndexOnBlob = ClassDDL.NewStd(mysql.ErrFunctionalIndexOnBlob)
// ErrDependentByPartitionFunctional returns when the dropped column depends by expression partition.
ErrDependentByPartitionFunctional = ClassDDL.NewStd(mysql.ErrDependentByPartitionFunctional)

// ErrUnsupportedAlterTableSpec means we don't support this alter table specification (i.e. unknown)
ErrUnsupportedAlterTableSpec = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "Unsupported/unknown ALTER TABLE specification"), nil))
Expand Down

0 comments on commit 83d275c

Please sign in to comment.