Skip to content

Commit

Permalink
resolve the conflicts
Browse files Browse the repository at this point in the history 
Signed-off-by: AilinKid <314806019@qq.com>
  • Loading branch information
@AilinKid
AilinKid committed Nov 17, 2020
1 parent a52e9d6 commit b3bccc6
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 176 deletions.
80 changes: 0 additions & 80 deletions ddl/column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,93 +352,13 @@ 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{}
pos := &ast.ColumnPosition{}
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)
Expand Down
6 changes: 1 addition & 5 deletions ddl/column_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")},
Expand Down
19 changes: 19 additions & 0 deletions ddl/db_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
24 changes: 2 additions & 22 deletions ddl/ddl_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
69 changes: 0 additions & 69 deletions ddl/ddl_worker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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
Expand Down Expand Up @@ -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")}}
Expand Down

0 comments on commit b3bccc6

Please sign in to comment.