From 5434d8046edcec19c0bbf5354cffe780829efb5a Mon Sep 17 00:00:00 2001 From: Ewan Chou Date: Tue, 18 Apr 2017 20:50:20 +0800 Subject: [PATCH] ddl: add the reason for the unsupported modify column error. (#3078) --- ddl/column_test.go | 25 +++++++++++++++---------- ddl/ddl.go | 2 +- ddl/ddl_api.go | 45 ++++++++++++++++++++++++++------------------- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/ddl/column_test.go b/ddl/column_test.go index 7eefe7371e..03cb83f670 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -903,21 +903,26 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { cases := []struct { origin string to string - ok bool + err error }{ - {"int", "bigint", true}, - {"int", "int unsigned", false}, - {"varchar(10)", "text", true}, - {"varbinary(10)", "blob", true}, - {"text", "blob", false}, - {"varchar(10)", "varchar(8)", false}, - {"varchar(10)", "varchar(11)", true}, - {"varchar(10) character set utf8 collate utf8_bin", "varchar(10) character set utf8", true}, + {"int", "bigint", nil}, + {"int", "int unsigned", errUnsupportedModifyColumn.GenByArgs("unsigned true not match origin false")}, + {"varchar(10)", "text", nil}, + {"varbinary(10)", "blob", nil}, + {"text", "blob", errUnsupportedModifyColumn.GenByArgs("charset binary not match origin utf8")}, + {"varchar(10)", "varchar(8)", errUnsupportedModifyColumn.GenByArgs("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}, } for _, ca := range cases { ftA := s.colDefStrToFieldType(c, ca.origin) ftB := s.colDefStrToFieldType(c, ca.to) - c.Assert(modifiable(ftA, ftB), Equals, ca.ok) + err := modifiable(ftA, ftB) + if err == nil { + c.Assert(ca.err, IsNil) + } else { + c.Assert(err.Error(), Equals, ca.err.Error()) + } } } diff --git a/ddl/ddl.go b/ddl/ddl.go index 24501fc358..403b3e67cc 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -51,7 +51,7 @@ var ( // We don't support dropping column with index covered now. errCantDropColWithIndex = terror.ClassDDL.New(codeCantDropColWithIndex, "can't drop column with index") errUnsupportedAddColumn = terror.ClassDDL.New(codeUnsupportedAddColumn, "unsupported add column") - errUnsupportedModifyColumn = terror.ClassDDL.New(codeUnsupportedModifyColumn, "unsupported modify column") + errUnsupportedModifyColumn = terror.ClassDDL.New(codeUnsupportedModifyColumn, "unsupported modify column %s") errUnsupportedPKHandle = terror.ClassDDL.New(codeUnsupportedDropPKHandle, "unsupported drop integer primary key") diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 8f318f0dea..24c2dca414 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -896,18 +896,28 @@ func (d *ddl) DropColumn(ctx context.Context, ti ast.Ident, colName model.CIStr) // change or check existing data in the table. // 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) bool { +func modifiable(origin *types.FieldType, to *types.FieldType) error { if to.Flen > 0 && to.Flen < origin.Flen { - return false + msg := fmt.Sprintf("length %d is less than origin %d", to.Flen, origin.Flen) + return errUnsupportedModifyColumn.GenByArgs(msg) } if to.Decimal > 0 && to.Decimal < origin.Decimal { - return false + msg := fmt.Sprintf("decimal %d is less than origin %d", to.Decimal, origin.Decimal) + return errUnsupportedModifyColumn.GenByArgs(msg) } - if origin.Charset != to.Charset || origin.Collate != to.Collate { - return false + if to.Charset != origin.Charset { + msg := fmt.Sprintf("charset %s not match origin %s", to.Charset, origin.Charset) + return errUnsupportedModifyColumn.GenByArgs(msg) } - if mysql.HasUnsignedFlag(uint(origin.Flag)) != mysql.HasUnsignedFlag(uint(to.Flag)) { - return false + if to.Collate != origin.Collate { + msg := fmt.Sprintf("collate %s not match origin %s", to.Collate, origin.Collate) + return errUnsupportedModifyColumn.GenByArgs(msg) + } + toUnsigned := mysql.HasUnsignedFlag(uint(to.Flag)) + originUnsigned := mysql.HasUnsignedFlag(uint(origin.Flag)) + if originUnsigned != toUnsigned { + msg := fmt.Sprintf("unsigned %v not match origin %v", toUnsigned, originUnsigned) + return errUnsupportedModifyColumn.GenByArgs(msg) } switch origin.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, @@ -915,24 +925,20 @@ func modifiable(origin *types.FieldType, to *types.FieldType) bool { switch to.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: - return true - default: - return false + return nil } 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 true - default: - return false + return nil } default: if origin.Tp == to.Tp { - return true + return nil } - - return false } + msg := fmt.Sprintf("type %v not match orgin %v", to.Tp, origin.Tp) + return errUnsupportedModifyColumn.GenByArgs(msg) } func setDefaultValue(ctx context.Context, col *table.Column, option *ast.ColumnOption) error { @@ -1029,15 +1035,16 @@ func (d *ddl) getModifiableColumnJob(ctx context.Context, ident ast.Ident, origi FieldType: *spec.NewColumn.Tp, } setCharsetCollationFlenDecimal(&newCol.FieldType) - if !modifiable(&col.FieldType, &newCol.FieldType) { - return nil, errors.Trace(errUnsupportedModifyColumn) + err = modifiable(&col.FieldType, &newCol.FieldType) + if err != nil { + return nil, errors.Trace(err) } if err := setDefaultAndComment(ctx, newCol, spec.NewColumn.Options); err != nil { return nil, errors.Trace(err) } // We don't support modifying the type definitions from 'null' to 'not null' now. if !mysql.HasNotNullFlag(col.Flag) && mysql.HasNotNullFlag(newCol.Flag) { - return nil, errors.Trace(errUnsupportedModifyColumn) + return nil, errUnsupportedModifyColumn.GenByArgs("null to not null") } newCol.Name = spec.NewColumn.Name.Name