diff --git a/ddl/column.go b/ddl/column.go index ddd13aa14b5cf..a35d2740a0f96 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -352,7 +352,6 @@ func onSetDefaultValue(t *meta.Meta, job *model.Job) (ver int64, _ error) { return updateColumnDefaultValue(t, job, newCol, &newCol.Name) } -<<<<<<< HEAD func (w *worker) onModifyColumn(t *meta.Meta, job *model.Job) (ver int64, _ error) { newCol := &model.ColumnInfo{} oldColName := &model.CIStr{} @@ -360,85 +359,6 @@ func (w *worker) onModifyColumn(t *meta.Meta, job *model.Job) (ver int64, _ erro var modifyColumnTp byte var updatedAutoRandomBits uint64 err := job.DecodeArgs(newCol, oldColName, pos, &modifyColumnTp, &updatedAutoRandomBits) -======= -func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { - toUnsigned := mysql.HasUnsignedFlag(newCol.Flag) - originUnsigned := mysql.HasUnsignedFlag(oldCol.Flag) - needTruncationOrToggleSign := func() bool { - return (newCol.Flen > 0 && newCol.Flen < oldCol.Flen) || (toUnsigned != originUnsigned) - } - // Ignore the potential max display length represented by integer's flen, use default flen instead. - oldColFlen, _ := mysql.GetDefaultFieldLengthAndDecimal(oldCol.Tp) - newColFlen, _ := mysql.GetDefaultFieldLengthAndDecimal(newCol.Tp) - needTruncationOrToggleSignForInteger := func() bool { - return (newColFlen > 0 && newColFlen < oldColFlen) || (toUnsigned != originUnsigned) - } - - // Deal with the same type. - if oldCol.Tp == newCol.Tp { - switch oldCol.Tp { - case mysql.TypeNewDecimal: - // Since type decimal will encode the precision, frac, negative(signed) and wordBuf into storage together, there is no short - // cut to eliminate data reorg change for column type change between decimal. - return oldCol.Flen != newCol.Flen || oldCol.Decimal != newCol.Decimal || toUnsigned != originUnsigned - case mysql.TypeEnum, mysql.TypeSet: - return isElemsChangedToModifyColumn(oldCol.Elems, newCol.Elems) - case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: - return toUnsigned != originUnsigned - } - - return needTruncationOrToggleSign() - } - - // Deal with the different type. - switch oldCol.Tp { - case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: - switch newCol.Tp { - case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: - return needTruncationOrToggleSign() - } - case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: - switch newCol.Tp { - case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: - return needTruncationOrToggleSignForInteger() - } - case mysql.TypeFloat, mysql.TypeDouble: - switch newCol.Tp { - case mysql.TypeFloat, mysql.TypeDouble: - return needTruncationOrToggleSign() - } - } - - return true -} - -func isElemsChangedToModifyColumn(oldElems, newElems []string) bool { - if len(newElems) < len(oldElems) { - return true - } - for index, oldElem := range oldElems { - newElem := newElems[index] - if oldElem != newElem { - return true - } - } - return false -} - -type modifyColumnJobParameter struct { - newCol *model.ColumnInfo - oldColName *model.CIStr - modifyColumnTp byte - updatedAutoRandomBits uint64 - changingCol *model.ColumnInfo - changingIdxs []*model.IndexInfo - pos *ast.ColumnPosition -} - -func getModifyColumnInfo(t *meta.Meta, job *model.Job) (*model.DBInfo, *model.TableInfo, *model.ColumnInfo, *modifyColumnJobParameter, error) { - jobParam := &modifyColumnJobParameter{pos: &ast.ColumnPosition{}} - err := job.DecodeArgs(&jobParam.newCol, &jobParam.oldColName, jobParam.pos, &jobParam.modifyColumnTp, &jobParam.updatedAutoRandomBits, &jobParam.changingCol, &jobParam.changingIdxs) ->>>>>>> 38f876044... ddl: ignore integer zerofill size attribute when changing the column types (#20862) if err != nil { job.State = model.JobStateCancelled return ver, errors.Trace(err) diff --git a/ddl/column_test.go b/ddl/column_test.go index 1ac16bbcf8d5a..7dabc57219b1d 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -938,11 +938,7 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { err error }{ {"int", "bigint", nil}, -<<<<<<< HEAD - {"int", "int unsigned", errUnsupportedModifyColumn.GenWithStackByArgs("length 10 is less than origin 11")}, -======= - {"int", "int unsigned", errUnsupportedModifyColumn.GenWithStackByArgs("can't change unsigned integer to signed or vice versa, and tidb_enable_change_column_type is false")}, ->>>>>>> 38f876044... ddl: ignore integer zerofill size attribute when changing the column types (#20862) + {"int", "int unsigned", errUnsupportedModifyColumn.GenWithStackByArgs("can't change unsigned integer to signed or vice versa")}, {"varchar(10)", "text", nil}, {"varbinary(10)", "blob", nil}, {"text", "blob", errUnsupportedModifyCharset.GenWithStackByArgs("charset from utf8mb4 to binary")}, diff --git a/ddl/db_test.go b/ddl/db_test.go index b401bd7d9a346..879692f8696d7 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -4930,3 +4930,22 @@ func init() { domain.SchemaOutOfDateRetryInterval = int64(50 * time.Millisecond) domain.SchemaOutOfDateRetryTimes = int32(50) } + +// Test issue #20529. +func (s *testSerialDBSuite) TestColumnTypeChangeIgnoreDisplayLength(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + + // Change int to int(3). + // Although display length is increased, the default flen is decreased, reorg is needed. + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a int)") + tk.MustExec("alter table t modify column a int(3)") + + // Change int to bigint(1) + // Although display length is decreased, default flen is the same, reorg is not needed. + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a int)") + tk.MustExec("alter table t modify column a bigint(1)") + tk.MustExec("drop table if exists t") +} diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index dbf27a46ef4e3..cb2e509831d5a 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2704,16 +2704,12 @@ func checkModifyCharsetAndCollation(toCharset, toCollate, origCharset, origColla // CheckModifyTypeCompatible checks whether changes column type to another is compatible considering // field length and precision. -<<<<<<< HEAD func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) error { -======= -func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (allowedChangeColumnValueMsg string, err error) { + unsupportedMsg := fmt.Sprintf("type %v not match origin %v", to.CompactStr(), origin.CompactStr()) var ( toFlen = to.Flen originFlen = origin.Flen ) ->>>>>>> 38f876044... ddl: ignore integer zerofill size attribute when changing the column types (#20862) - 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: @@ -2726,16 +2722,10 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al 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: -<<<<<<< HEAD -======= // For integers, we should ignore the potential display length represented by flen, using // the default flen of the type. originFlen, _ = mysql.GetDefaultFieldLengthAndDecimal(origin.Tp) toFlen, _ = mysql.GetDefaultFieldLengthAndDecimal(to.Tp) - // Changing integer to integer, whether reorg is necessary is depend on the flen/decimal/signed. - skipSignCheck = true - skipLenCheck = true ->>>>>>> 38f876044... ddl: ignore integer zerofill size attribute when changing the column types (#20862) default: return errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } @@ -2774,19 +2764,9 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al return errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } } - -<<<<<<< HEAD - 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 toFlen > 0 && toFlen < originFlen { msg := fmt.Sprintf("length %d is less than origin %d", toFlen, originFlen) - if skipLenCheck { - return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) - } - return "", errUnsupportedModifyColumn.GenWithStackByArgs(msg) ->>>>>>> 38f876044... ddl: ignore integer zerofill size attribute when changing the column types (#20862) + 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) diff --git a/ddl/ddl_worker_test.go b/ddl/ddl_worker_test.go index 04a408f66eb77..8a96101ab3010 100644 --- a/ddl/ddl_worker_test.go +++ b/ddl/ddl_worker_test.go @@ -439,7 +439,6 @@ func buildCancelJobTests(firstID int64) []testCancelJob { {act: model.ActionShardRowID, jobIDs: []int64{firstID + 17}, cancelRetErrs: noErrs, cancelState: model.StateNone}, {act: model.ActionModifyColumn, jobIDs: []int64{firstID + 18}, cancelRetErrs: noErrs, cancelState: model.StateNone}, -<<<<<<< HEAD {act: model.ActionAddForeignKey, jobIDs: []int64{firstID + 19}, cancelRetErrs: noErrs, cancelState: model.StateNone}, {act: model.ActionAddForeignKey, jobIDs: []int64{firstID + 20}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 20)}, cancelState: model.StatePublic}, {act: model.ActionDropForeignKey, jobIDs: []int64{firstID + 21}, cancelRetErrs: noErrs, cancelState: model.StateNone}, @@ -465,57 +464,6 @@ func buildCancelJobTests(firstID int64) []testCancelJob { {act: model.ActionAddTablePartition, jobIDs: []int64{firstID + 42}, cancelRetErrs: noErrs, cancelState: model.StateNone}, {act: model.ActionAddTablePartition, jobIDs: []int64{firstID + 43}, cancelRetErrs: noErrs, cancelState: model.StateReplicaOnly}, {act: model.ActionAddTablePartition, jobIDs: []int64{firstID + 44}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob}, cancelState: model.StatePublic}, -======= - {act: model.ActionModifyColumn, jobIDs: []int64{firstID + 19}, cancelRetErrs: noErrs, cancelState: model.StateDeleteOnly}, - - {act: model.ActionAddForeignKey, jobIDs: []int64{firstID + 20}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionAddForeignKey, jobIDs: []int64{firstID + 21}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 21)}, cancelState: model.StatePublic}, - {act: model.ActionDropForeignKey, jobIDs: []int64{firstID + 22}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionDropForeignKey, jobIDs: []int64{firstID + 23}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 23)}, cancelState: model.StatePublic}, - - {act: model.ActionRenameTable, jobIDs: []int64{firstID + 24}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionRenameTable, jobIDs: []int64{firstID + 25}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 25)}, cancelState: model.StatePublic}, - - {act: model.ActionModifyTableCharsetAndCollate, jobIDs: []int64{firstID + 26}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionModifyTableCharsetAndCollate, jobIDs: []int64{firstID + 27}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 27)}, cancelState: model.StatePublic}, - {act: model.ActionTruncateTablePartition, jobIDs: []int64{firstID + 28}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionTruncateTablePartition, jobIDs: []int64{firstID + 29}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 29)}, cancelState: model.StatePublic}, - {act: model.ActionModifySchemaCharsetAndCollate, jobIDs: []int64{firstID + 31}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionModifySchemaCharsetAndCollate, jobIDs: []int64{firstID + 32}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 32)}, cancelState: model.StatePublic}, - - {act: model.ActionAddPrimaryKey, jobIDs: []int64{firstID + 33}, cancelRetErrs: noErrs, cancelState: model.StateDeleteOnly}, - {act: model.ActionAddPrimaryKey, jobIDs: []int64{firstID + 34}, cancelRetErrs: noErrs, cancelState: model.StateWriteOnly}, - {act: model.ActionAddPrimaryKey, jobIDs: []int64{firstID + 35}, cancelRetErrs: noErrs, cancelState: model.StateWriteReorganization}, - {act: model.ActionAddPrimaryKey, jobIDs: []int64{firstID + 36}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 36)}, cancelState: model.StatePublic}, - {act: model.ActionDropPrimaryKey, jobIDs: []int64{firstID + 37}, cancelRetErrs: noErrs, cancelState: model.StateWriteOnly}, - {act: model.ActionDropPrimaryKey, jobIDs: []int64{firstID + 38}, cancelRetErrs: []error{admin.ErrCannotCancelDDLJob.GenWithStackByArgs(firstID + 38)}, cancelState: model.StateDeleteOnly}, - - {act: model.ActionAddColumns, jobIDs: []int64{firstID + 39}, cancelRetErrs: noErrs, cancelState: model.StateDeleteOnly}, - {act: model.ActionAddColumns, jobIDs: []int64{firstID + 40}, cancelRetErrs: noErrs, cancelState: model.StateWriteOnly}, - {act: model.ActionAddColumns, jobIDs: []int64{firstID + 41}, cancelRetErrs: noErrs, cancelState: model.StateWriteReorganization}, - {act: model.ActionAddColumns, jobIDs: []int64{firstID + 42}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 42)}, cancelState: model.StatePublic}, - - {act: model.ActionDropColumns, jobIDs: []int64{firstID + 43}, cancelRetErrs: []error{admin.ErrCannotCancelDDLJob.GenWithStackByArgs(firstID + 43)}, cancelState: model.StateDeleteOnly}, - {act: model.ActionDropColumns, jobIDs: []int64{firstID + 44}, cancelRetErrs: []error{admin.ErrCannotCancelDDLJob.GenWithStackByArgs(firstID + 44)}, cancelState: model.StateWriteOnly}, - {act: model.ActionDropColumns, jobIDs: []int64{firstID + 45}, cancelRetErrs: []error{admin.ErrCannotCancelDDLJob.GenWithStackByArgs(firstID + 45)}, cancelState: model.StateWriteReorganization}, - - {act: model.ActionAlterIndexVisibility, jobIDs: []int64{firstID + 47}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionAlterIndexVisibility, jobIDs: []int64{firstID + 48}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 48)}, cancelState: model.StatePublic}, - - {act: model.ActionExchangeTablePartition, jobIDs: []int64{firstID + 54}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionExchangeTablePartition, jobIDs: []int64{firstID + 55}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob.GenWithStackByArgs(firstID + 55)}, cancelState: model.StatePublic}, - - {act: model.ActionAddTablePartition, jobIDs: []int64{firstID + 60}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionAddTablePartition, jobIDs: []int64{firstID + 61}, cancelRetErrs: noErrs, cancelState: model.StateReplicaOnly}, - {act: model.ActionAddTablePartition, jobIDs: []int64{firstID + 62}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob}, cancelState: model.StatePublic}, - - // modify column has two different types, normal-type and reorg-type. The latter has 5 states and it can be cancelled except the public state. - {act: model.ActionModifyColumn, jobIDs: []int64{firstID + 65}, cancelRetErrs: noErrs, cancelState: model.StateNone}, - {act: model.ActionModifyColumn, jobIDs: []int64{firstID + 66}, cancelRetErrs: noErrs, cancelState: model.StateDeleteOnly}, - {act: model.ActionModifyColumn, jobIDs: []int64{firstID + 67}, cancelRetErrs: noErrs, cancelState: model.StateWriteOnly}, - {act: model.ActionModifyColumn, jobIDs: []int64{firstID + 68}, cancelRetErrs: noErrs, cancelState: model.StateWriteReorganization}, - {act: model.ActionModifyColumn, jobIDs: []int64{firstID + 69}, cancelRetErrs: []error{admin.ErrCancelFinishedDDLJob}, cancelState: model.StatePublic}, ->>>>>>> 38f876044... ddl: ignore integer zerofill size attribute when changing the column types (#20862) } return tests @@ -782,23 +730,6 @@ func (s *testDDLSuite) TestCancelJob(c *C) { changedCol := model.FindColumnInfo(changedTable.Meta().Columns, col.Name.L) c.Assert(changedCol.DefaultValue, IsNil) -<<<<<<< HEAD -======= - // modify delete-only-state column, - col.FieldType.Tp = mysql.TypeTiny - col.FieldType.Flen = col.FieldType.Flen - 1 - updateTest(&tests[16]) - modifyColumnArgs = []interface{}{col, col.Name, &ast.ColumnPosition{}, byte(0), uint64(0)} - cancelState = model.StateNone - doDDLJobErrWithSchemaState(ctx, d, c, dbInfo.ID, tblInfo.ID, test.act, modifyColumnArgs, &cancelState) - c.Check(checkErr, IsNil) - changedTable = testGetTable(c, d, dbInfo.ID, tblInfo.ID) - changedCol = model.FindColumnInfo(changedTable.Meta().Columns, col.Name.L) - c.Assert(changedCol.FieldType.Tp, Equals, mysql.TypeLonglong) - c.Assert(changedCol.FieldType.Flen, Equals, col.FieldType.Flen+1) - col.FieldType.Flen++ - ->>>>>>> 38f876044... ddl: ignore integer zerofill size attribute when changing the column types (#20862) // Test add foreign key failed cause by canceled. updateTest(&tests[16]) addForeignKeyArgs := []interface{}{model.FKInfo{Name: model.NewCIStr("fk1")}}