ddl: update the error message of "modify/change column" to make it ea…

…sier to understand (#13457)
pingcap · Nov 28, 2019 · 6bc298e · 6bc298e
1 parent bcf2f07
commit 6bc298e
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 46 deletions.
diff --git a/ddl/column_test.go b/ddl/column_test.go
@@ -934,7 +934,8 @@ func (s *testColumnSuite) TestModifyColumn(c *C) {
 		{"varchar(10)", "varchar(8)", errUnsupportedModifyColumn.GenWithStackByArgs("length 8 is less than origin 10")},
 		{"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.GenWithStack("unsupported modify decimal column precision")},
+		{"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
@@ -470,7 +470,7 @@ func (s *testStateChangeSuite) TestAppendEnum(c *C) {
 	c.Assert(err.Error(), Equals, "[ddl:203]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:210]unsupported modify charset from utf8 to binary")
+	c.Assert(err.Error(), Equals, "[ddl:203]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
@@ -818,9 +818,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 *testIntegrationSuite1) TestIndexOnMultipleGeneratedColumn(c *C) {

diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go
@@ -2372,65 +2372,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.GenWithStack("unsupported modify 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("unsigned %v not match origin %v", toUnsigned, originUnsigned)
-		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("cannot modify enum column value %s to %s", originElem, toElem)
-					return errUnsupportedModifyColumn.GenWithStackByArgs(msg)
-				}
-			}
-			return nil
 		}
-		msg := fmt.Sprintf("cannot modify enum type column's to type %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 %v not match origin %v", to.Tp, origin.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) {

diff --git a/executor/admin_test.go b/executor/admin_test.go
@@ -713,7 +713,7 @@ func (s *testSuite1) TestAdminCheckTable(c *C) {
 	tk.MustExec(`insert into t1 set a='1.9'`)
 	err = tk.ExecToErr(`alter table t1 modify column a decimal(3,2);`)
 	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "[ddl:203]unsupported modify decimal column precision")
+	c.Assert(err.Error(), Equals, "[ddl:203]unsupported modify column can't change decimal column precision")
 	tk.MustExec(`delete from t1;`)
 	tk.MustExec(`admin check table t1;`)
 }