diff --git a/ddl/column.go b/ddl/column.go index cf4d55c20e034..e4b7dd0a6815a 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -652,10 +652,29 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { // cut to eliminate data reorg change for column type change between decimal. return oldCol.Flen != newCol.Flen || oldCol.Decimal != newCol.Decimal || toUnsigned != originUnsigned } + if oldCol.Tp == newCol.Tp && (oldCol.Tp == mysql.TypeEnum || oldCol.Tp == mysql.TypeSet) { + return isElemsChangedToModifyColumn(oldCol.Elems, newCol.Elems) + } + if oldCol.Tp == mysql.TypeEnum || oldCol.Tp == mysql.TypeSet || + newCol.Tp == mysql.TypeEnum || newCol.Tp == mysql.TypeSet { + return true + } if newCol.Flen > 0 && newCol.Flen < oldCol.Flen || toUnsigned != originUnsigned { return true } + return false +} +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 } @@ -901,7 +920,7 @@ func (w *worker) doModifyColumnTypeWithData( // If timeout, we should return, check for the owner and re-wait job done. return ver, nil } - if kv.ErrKeyExists.Equal(err) || errCancelledDDLJob.Equal(err) || errCantDecodeRecord.Equal(err) || types.ErrOverflow.Equal(err) { + if needRollbackData(err) { if err1 := t.RemoveDDLReorgHandle(job, reorgInfo.elements); err1 != nil { logutil.BgLogger().Warn("[ddl] run modify column job failed, RemoveDDLReorgHandle failed, can't convert job to rollback", zap.String("job", job.String()), zap.Error(err1)) @@ -960,6 +979,12 @@ func (w *worker) doModifyColumnTypeWithData( return ver, errors.Trace(err) } +// needRollbackData indicates whether it needs to rollback data when specific error occurs. +func needRollbackData(err error) bool { + return kv.ErrKeyExists.Equal(err) || errCancelledDDLJob.Equal(err) || errCantDecodeRecord.Equal(err) || + types.ErrOverflow.Equal(err) || types.ErrDataTooLong.Equal(err) || types.ErrTruncated.Equal(err) +} + // BuildElements is exported for testing. func BuildElements(changingCol *model.ColumnInfo, changingIdxs []*model.IndexInfo) []*meta.Element { elements := make([]*meta.Element, 0, len(changingIdxs)+1) @@ -1148,6 +1173,7 @@ func (w *updateColumnWorker) getRowRecord(handle kv.Handle, recordKey []byte, ra var recordWarning *terror.Error newColVal, err := table.CastValue(w.sessCtx, w.rowMap[w.oldColInfo.ID], w.newColInfo, false, false) if err != nil { + err = w.reformatErrors(err) if IsNormalWarning(err) || (!w.sqlMode.HasStrictMode() && IsStrictWarning(err)) { // Keep the warnings. recordWarning = errors.Cause(err).(*terror.Error) @@ -1182,6 +1208,15 @@ func (w *updateColumnWorker) getRowRecord(handle kv.Handle, recordKey []byte, ra return nil } +// reformatErrors casted error because `convertTo` function couldn't package column name and datum value for some errors. +func (w *updateColumnWorker) reformatErrors(err error) error { + // Since row count is not precious in concurrent reorganization, here we substitute row count with datum value. + if types.ErrTruncated.Equal(err) { + err = types.ErrTruncated.GenWithStack("Data truncated for column '%s', value is '%s'", w.oldColInfo.Name, w.rowMap[w.oldColInfo.ID]) + } + return err +} + // IsNormalWarning is used to check the normal warnings, for example data-truncated warnings. // This kind of warning will be always thrown out regard less of what kind of the sql mode is. func IsNormalWarning(err error) bool { @@ -1196,7 +1231,7 @@ func IsNormalWarning(err error) bool { // non-strict SQL Mode. func IsStrictWarning(err error) bool { // TODO: there are more errors here can be identified as warnings under non-strict SQL mode. - if types.ErrOverflow.Equal(err) { + if types.ErrOverflow.Equal(err) || types.ErrTruncated.Equal(err) { return true } return false diff --git a/ddl/column_test.go b/ddl/column_test.go index 9c7679a12741b..1ce4d641f56c7 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -1156,7 +1156,7 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { {"varchar(10)", "text", nil}, {"varbinary(10)", "blob", nil}, {"text", "blob", errUnsupportedModifyCharset.GenWithStackByArgs("charset from utf8mb4 to binary")}, - {"varchar(10)", "varchar(8)", errUnsupportedModifyColumn.GenWithStackByArgs("length 8 is less than origin 10")}, + {"varchar(10)", "varchar(8)", errUnsupportedModifyColumn.GenWithStackByArgs("length 8 is less than origin 10, and tidb_enable_change_column_type is false")}, {"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("decimal change from decimal(2, 1) to decimal(3, 2), and tidb_enable_change_column_type is false")}, diff --git a/ddl/db_change_test.go b/ddl/db_change_test.go index 516564d719ad7..1e34778dadea8 100644 --- a/ddl/db_change_test.go +++ b/ddl/db_change_test.go @@ -479,10 +479,9 @@ func (s *testStateChangeSuite) TestAppendEnum(c *C) { c.Assert(err, IsNil) _, err = s.se.Execute(context.Background(), "insert into t values('a', 'A', '2018-09-19', 9)") - c.Assert(err.Error(), Equals, "[types:1265]Data truncated for column 'c2' at row 1") failAlterTableSQL1 := "alter table t change c2 c2 enum('N') DEFAULT 'N'" _, err = s.se.Execute(context.Background(), failAlterTableSQL1) - c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: the number of enum column's elements is less than the original: 2") + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: the number of enum column's elements is less than the original: 2, and tidb_enable_change_column_type is false") 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 column: cannot modify enum type column's to type int(11)") diff --git a/ddl/db_test.go b/ddl/db_test.go index 1e1848b44fcba..5b395ebf760f4 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3856,6 +3856,7 @@ func (s *testSerialDBSuite) TestModifyColumnNullToNotNullWithChangingVal2(c *C) failpoint.Disable("github.com/pingcap/tidb/ddl/mockInsertValueAfterCheckNull") }() + tk.MustExec("drop table if exists tt;") tk.MustExec(`create table tt (a bigint, b int, unique index idx(a));`) tk.MustExec("insert into tt values (1,1),(2,2),(3,3);") _, err := tk.Exec("alter table tt modify a int not null;") @@ -3877,6 +3878,65 @@ func (s *testSerialDBSuite) TestModifyColumnNullToNotNullWithChangingVal(c *C) { c.Assert(c2.FieldType.Tp, Equals, mysql.TypeTiny) } +func (s *testSerialDBSuite) TestModifyColumnBetweenStringTypes(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.Se.GetSessionVars().EnableChangeColumnType = true + + //varchar to varchar + tk.MustExec("drop table if exists tt;") + tk.MustExec("create table tt (a varchar(10));") + tk.MustExec("insert into tt values ('111'),('10000');") + tk.MustExec("alter table tt change a a varchar(5);") + c2 := getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(c2.FieldType.Flen, Equals, 5) + tk.MustQuery("select * from tt").Check(testkit.Rows("111", "10000")) + tk.MustGetErrMsg("alter table tt change a a varchar(4);", "[types:1406]Data Too Long, field len 4, data len 5") + tk.MustExec("alter table tt change a a varchar(100);") + + // varchar to char + tk.MustExec("alter table tt change a a char(10);") + c2 = getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(c2.FieldType.Tp, Equals, mysql.TypeString) + c.Assert(c2.FieldType.Flen, Equals, 10) + tk.MustQuery("select * from tt").Check(testkit.Rows("111", "10000")) + tk.MustGetErrMsg("alter table tt change a a char(4);", "[types:1406]Data Too Long, field len 4, data len 5") + + // char to text + tk.MustExec("alter table tt change a a text;") + c2 = getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(c2.FieldType.Tp, Equals, mysql.TypeBlob) + + // text to set + tk.MustGetErrMsg("alter table tt change a a set('111', '2222');", "[types:1265]Data truncated for column 'a', value is 'KindBytes 10000'") + tk.MustExec("alter table tt change a a set('111', '10000');") + c2 = getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(c2.FieldType.Tp, Equals, mysql.TypeSet) + tk.MustQuery("select * from tt").Check(testkit.Rows("111", "10000")) + + // set to set + tk.MustExec("alter table tt change a a set('10000', '111');") + c2 = getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(c2.FieldType.Tp, Equals, mysql.TypeSet) + tk.MustQuery("select * from tt").Check(testkit.Rows("111", "10000")) + + // set to enum + tk.MustGetErrMsg("alter table tt change a a enum('111', '2222');", "[types:1265]Data truncated for column 'a', value is 'KindMysqlSet 10000'") + tk.MustExec("alter table tt change a a enum('111', '10000');") + c2 = getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(c2.FieldType.Tp, Equals, mysql.TypeEnum) + tk.MustQuery("select * from tt").Check(testkit.Rows("111", "10000")) + tk.MustExec("alter table tt change a a enum('10000', '111');") + tk.MustQuery("select * from tt where a = 1").Check(testkit.Rows("10000")) + tk.MustQuery("select * from tt where a = 2").Check(testkit.Rows("111")) + + // no-strict mode + tk.MustExec(`set @@sql_mode="";`) + tk.MustExec("alter table tt change a a enum('111', '2222');") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1265|Data truncated for column 'a', value is 'KindMysqlEnum 10000'")) + + tk.MustExec("drop table tt;") +} + func getModifyColumn(c *C, ctx sessionctx.Context, db, tbl, colName string, allColumn bool) *table.Column { t := testGetTableByName(c, ctx, db, tbl) colName = strings.ToLower(colName) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index d19d031f50932..8e79ab790ad13 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3336,20 +3336,23 @@ func checkModifyCharsetAndCollation(toCharset, toCollate, origCharset, origColla // field length and precision. 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 isIntType bool + var canChange bool switch origin.Tp { - case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, - mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: + 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: + canChange = true + case mysql.TypeEnum, mysql.TypeSet: + return unsupportedMsg, errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) 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: - isIntType = true + canChange = true default: return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } @@ -3360,21 +3363,27 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al } else { typeVar = "set" } - if origin.Tp != to.Tp { + switch to.Tp { + case mysql.TypeEnum, mysql.TypeSet: + if len(to.Elems) < len(origin.Elems) { + msg := fmt.Sprintf("the number of %s column's elements is less than the original: %d", typeVar, len(origin.Elems)) + return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } + for index, originElem := range origin.Elems { + toElem := to.Elems[index] + if originElem != toElem { + msg := fmt.Sprintf("cannot modify %s column value %s to %s", typeVar, originElem, toElem) + return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } + } + case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, + mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: + msg := fmt.Sprintf("cannot modify %s type column's to type %s", typeVar, to.String()) + return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) + default: msg := fmt.Sprintf("cannot modify %s type column's to type %s", typeVar, to.String()) return "", errUnsupportedModifyColumn.GenWithStackByArgs(msg) } - if len(to.Elems) < len(origin.Elems) { - msg := fmt.Sprintf("the number of %s column's elements is less than the original: %d", typeVar, 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 %s column value %s to %s", typeVar, originElem, toElem) - return "", errUnsupportedModifyColumn.GenWithStackByArgs(msg) - } - } case mysql.TypeNewDecimal: if origin.Tp != to.Tp { return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) @@ -3394,7 +3403,7 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al if to.Flen > 0 && to.Flen < origin.Flen { msg := fmt.Sprintf("length %d is less than origin %d", to.Flen, origin.Flen) - if isIntType { + if canChange { return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) } return "", errUnsupportedModifyColumn.GenWithStackByArgs(msg) @@ -3408,7 +3417,7 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al originUnsigned := mysql.HasUnsignedFlag(origin.Flag) if originUnsigned != toUnsigned { msg := fmt.Sprintf("can't change unsigned integer to signed or vice versa") - if isIntType { + if canChange { return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) } return "", errUnsupportedModifyColumn.GenWithStackByArgs(msg) diff --git a/expression/integration_test.go b/expression/integration_test.go index a17ebac430338..3027557fb16e8 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -7482,7 +7482,8 @@ func (s *testIntegrationSerialSuite) TestIssue19804(c *C) { tk.MustExec(`drop table if exists t;`) tk.MustExec(`create table t(a set('a', 'b', 'c'));`) tk.MustExec(`alter table t change a a set('a', 'b', 'c', 'd');`) - tk.MustGetErrMsg(`alter table t change a a set('a', 'b', 'c', 'e', 'f');`, "[ddl:8200]Unsupported modify column: cannot modify set column value d to e") + tk.MustExec(`insert into t values('d');`) + tk.MustGetErrMsg(`alter table t change a a set('a', 'b', 'c', 'e', 'f');`, "[ddl:8200]Unsupported modify column: cannot modify set column value d to e, and tidb_enable_change_column_type is false") } func (s *testIntegrationSerialSuite) TestIssue18949(c *C) {