diff --git a/ddl/column_test.go b/ddl/column_test.go index 59dd05147a530..7e1b9641dbbe4 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -951,6 +951,7 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { {"varchar(10)", "varchar(11)", nil}, {"varchar(10) character set utf8 collate utf8_bin", "varchar(10) character set utf8", nil}, {"decimal(2,1)", "decimal(3,2)", errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision")}, + {"decimal(2,1)", "decimal(2,2)", errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision")}, {"decimal(2,1)", "decimal(2,1)", nil}, } for _, tt := range tests { diff --git a/ddl/db_change_test.go b/ddl/db_change_test.go index a56828cd1b400..35d473642c30d 100644 --- a/ddl/db_change_test.go +++ b/ddl/db_change_test.go @@ -470,7 +470,7 @@ func (s *testStateChangeSuite) TestAppendEnum(c *C) { c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: the number of enum column's elements is less than the original: 2") failAlterTableSQL2 := "alter table t change c2 c2 int default 0" _, err = s.se.Execute(context.Background(), failAlterTableSQL2) - c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify charset from utf8 to binary") + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: cannot modify enum type column's to type int(11)") alterTableSQL := "alter table t change c2 c2 enum('N','Y','A') DEFAULT 'A'" _, err = s.se.Execute(context.Background(), alterTableSQL) c.Assert(err, IsNil) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 1ce60d2b4c23f..d6755b3bb5afa 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -804,9 +804,19 @@ func (s *testIntegrationSuite5) TestModifyingColumnOption(c *C) { tk.MustExec("drop table t1") tk.MustExec("drop table if exists t2") - tk.MustExec("create table t1 (a int)") - tk.MustExec("create table t2 (b int, c int)") + tk.MustExec("create table t1 (a int(11) default null)") + tk.MustExec("create table t2 (b char, c int)") assertErrCode("alter table t2 modify column c int references t1(a)", errMsg) + _, err := tk.Exec("alter table t1 change a a varchar(16)") + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: type varchar(16) not match origin int(11)") + _, err = tk.Exec("alter table t1 change a a varchar(10)") + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: type varchar(10) not match origin int(11)") + _, err = tk.Exec("alter table t1 change a a datetime") + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: type datetime not match origin int(11)") + _, err = tk.Exec("alter table t1 change a a int(11) unsigned") + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: can't change unsigned integer to signed or vice versa") + _, err = tk.Exec("alter table t2 change b b int(11) unsigned") + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: type int(11) not match origin char(1)") } func (s *testIntegrationSuite4) TestIndexOnMultipleGeneratedColumn(c *C) { diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 7d0c7e1dd62bd..6bb5f1a0c9cb2 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2438,65 +2438,67 @@ func modifiableCharsetAndCollation(toCharset, toCollate, origCharset, origCollat // It returns true if the two types has the same Charset and Collation, the same sign, both are // integer types or string types, and new Flen and Decimal must be greater than or equal to origin. func modifiable(origin *types.FieldType, to *types.FieldType) error { - // The root cause is modifying decimal precision needs to rewrite binary representation of that decimal. - if origin.Tp == mysql.TypeNewDecimal && (to.Flen != origin.Flen || to.Decimal != origin.Decimal) { - return errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision") - } - if to.Flen > 0 && to.Flen < origin.Flen { - msg := fmt.Sprintf("length %d is less than origin %d", to.Flen, origin.Flen) - return errUnsupportedModifyColumn.GenWithStackByArgs(msg) - } - if to.Decimal > 0 && to.Decimal < origin.Decimal { - msg := fmt.Sprintf("decimal %d is less than origin %d", to.Decimal, origin.Decimal) - return errUnsupportedModifyColumn.GenWithStackByArgs(msg) - } - if err := modifiableCharsetAndCollation(to.Charset, to.Collate, origin.Charset, origin.Collate); err != nil { - return errors.Trace(err) - } - - toUnsigned := mysql.HasUnsignedFlag(to.Flag) - originUnsigned := mysql.HasUnsignedFlag(origin.Flag) - if originUnsigned != toUnsigned { - msg := fmt.Sprintf("can't change unsigned integer to signed or vice versa") - return errUnsupportedModifyColumn.GenWithStackByArgs(msg) - } + unsupportedMsg := fmt.Sprintf("type %v not match origin %v", to.CompactStr(), origin.CompactStr()) switch origin.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: switch to.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: - return nil + default: + return errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: switch to.Tp { case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: - return nil + default: + return errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } case mysql.TypeEnum: - if origin.Tp == to.Tp { - if len(to.Elems) < len(origin.Elems) { - msg := fmt.Sprintf("the number of enum column's elements is less than the original: %d", len(origin.Elems)) + if origin.Tp != to.Tp { + msg := fmt.Sprintf("cannot modify enum type column's to type %s", to.String()) + return errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } + if len(to.Elems) < len(origin.Elems) { + msg := fmt.Sprintf("the number of enum column's elements is less than the original: %d", len(origin.Elems)) + return errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } + for index, originElem := range origin.Elems { + toElem := to.Elems[index] + if originElem != toElem { + msg := fmt.Sprintf("cannot modify enum column value %s to %s", originElem, toElem) return errUnsupportedModifyColumn.GenWithStackByArgs(msg) } - for index, originElem := range origin.Elems { - toElem := to.Elems[index] - if originElem != toElem { - msg := fmt.Sprintf("can't change enum value %s to %s", originElem, toElem) - return errUnsupportedModifyColumn.GenWithStackByArgs(msg) - } - } - return nil } - msg := fmt.Sprintf("can't change enum type to %s", to.String()) - return errUnsupportedModifyColumn.GenWithStackByArgs(msg) + case mysql.TypeNewDecimal: + // The root cause is modifying decimal precision needs to rewrite binary representation of that decimal. + if to.Flen != origin.Flen || to.Decimal != origin.Decimal { + return errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision") + } default: - if origin.Tp == to.Tp { - return nil + if origin.Tp != to.Tp { + return errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } } - msg := fmt.Sprintf(":type not match, before: %v, after: %v", origin.Tp, to.Tp) - return errUnsupportedModifyColumn.GenWithStackByArgs(msg) + + if to.Flen > 0 && to.Flen < origin.Flen { + msg := fmt.Sprintf("length %d is less than origin %d", to.Flen, origin.Flen) + return errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } + if to.Decimal > 0 && to.Decimal < origin.Decimal { + msg := fmt.Sprintf("decimal %d is less than origin %d", to.Decimal, origin.Decimal) + return errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } + + toUnsigned := mysql.HasUnsignedFlag(to.Flag) + originUnsigned := mysql.HasUnsignedFlag(origin.Flag) + if originUnsigned != toUnsigned { + msg := fmt.Sprintf("can't change unsigned integer to signed or vice versa") + return errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } + + err := modifiableCharsetAndCollation(to.Charset, to.Collate, origin.Charset, origin.Collate) + return errors.Trace(err) } func setDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.ColumnOption) (bool, error) {