From 026364e7789e9c707e45c3c3ba0e321571135556 Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 16 Sep 2020 11:33:59 +0800 Subject: [PATCH 01/16] Supports transitions between the same type (String types) --- ddl/column.go | 2 +- ddl/ddl_api.go | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index ee725212fc4f7..d4b04138ec166 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -853,7 +853,7 @@ func (w *worker) doModifyColumnTypeWithData( logutil.BgLogger().Warn("[ddl] run modify column job failed, convert job to rollback", zap.String("job", job.String()), zap.Error(err)) // TODO: Do rollback. } - if types.ErrOverflow.Equal(err) { + if types.ErrOverflow.Equal(err) || types.ErrDataTooLong.Equal(err) { // TODO: Do rollback. job.State = model.JobStateCancelled return ver, errors.Trace(err) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 037defe59225f..b68df7624167f 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3240,20 +3240,21 @@ 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: switch to.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: + canChange = true 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) } @@ -3289,7 +3290,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) @@ -3303,7 +3304,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) From a4b223ce876d0db18d657fa53d1b6e071d8f8192 Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 16 Sep 2020 14:06:42 +0800 Subject: [PATCH 02/16] Supports transitions between the ENUM and Set and Strings type --- ddl/column.go | 39 +++++++++++++++++++++++++++++++++++++-- ddl/ddl_api.go | 33 ++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index d4b04138ec166..17813fd704636 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -647,7 +647,37 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { if newCol.Flen > 0 && newCol.Flen < oldCol.Flen || toUnsigned != originUnsigned { return true } - + if newCol.Tp == mysql.TypeEnum && oldCol.Tp == mysql.TypeEnum { + if len(newCol.Elems) < len(oldCol.Elems) { + return true + } + for index, originElem := range oldCol.Elems { + toElem := newCol.Elems[index] + if originElem != toElem { + return true + } + } + } + if newCol.Tp == mysql.TypeSet && oldCol.Tp == mysql.TypeSet { + if len(newCol.Elems) < len(oldCol.Elems) { + return true + } + for _, oldElem := range oldCol.Elems { + contain := false + for _, newElem := range newCol.Elems { + if oldElem == newElem { + contain = true + } + } + if !contain { + return true + } + } + } + if (newCol.Tp == mysql.TypeEnum || oldCol.Tp == mysql.TypeEnum || + newCol.Tp == mysql.TypeSet || oldCol.Tp == mysql.TypeSet) && oldCol.Tp != newCol.Tp { + return true + } return false } @@ -853,7 +883,7 @@ func (w *worker) doModifyColumnTypeWithData( logutil.BgLogger().Warn("[ddl] run modify column job failed, convert job to rollback", zap.String("job", job.String()), zap.Error(err)) // TODO: Do rollback. } - if types.ErrOverflow.Equal(err) || types.ErrDataTooLong.Equal(err) { + if types.ErrOverflow.Equal(err) || types.ErrDataTooLong.Equal(err) || types.ErrTruncated.Equal(err) { // TODO: Do rollback. job.State = model.JobStateCancelled return ver, errors.Trace(err) @@ -984,8 +1014,10 @@ func (w *updateColumnWorker) fetchRowColVals(txn kv.Transaction, taskRange reorg taskDone := false var lastAccessedHandle kv.Handle oprStartTime := startTime + var rowIdx int err := iterateSnapshotRows(w.sessCtx.GetStore(), w.priority, w.table, txn.StartTS(), taskRange.startHandle, taskRange.endHandle, taskRange.endIncluded, func(handle kv.Handle, recordKey kv.Key, rawRow []byte) (bool, error) { + rowIdx++ oprEndTime := time.Now() logSlowOperations(oprEndTime.Sub(oprStartTime), "iterateSnapshotRows in updateColumnWorker fetchRowColVals", 0) oprStartTime = oprEndTime @@ -1001,6 +1033,9 @@ func (w *updateColumnWorker) fetchRowColVals(txn kv.Transaction, taskRange reorg } if err1 := w.getRowRecord(handle, recordKey, rawRow); err1 != nil { + if types.ErrTruncated.Equal(err1) { + err1 = types.ErrTruncated.GenWithStackByArgs(w.oldColInfo.Name, rowIdx) + } return false, errors.Trace(err1) } lastAccessedHandle = handle diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index b68df7624167f..46e5a25e7a446 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3246,7 +3246,8 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al 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: + mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, + mysql.TypeEnum, mysql.TypeSet: canChange = true default: return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) @@ -3259,20 +3260,13 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } case mysql.TypeEnum: - 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) - } + switch to.Tp { + case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, + mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, + mysql.TypeSet, mysql.TypeEnum: + canChange = true + default: + return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } case mysql.TypeNewDecimal: if origin.Tp != to.Tp { @@ -3282,6 +3276,15 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al if to.Flen != origin.Flen || to.Decimal != origin.Decimal { return "", errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision") } + case mysql.TypeSet: + switch to.Tp { + case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, + mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, + mysql.TypeSet, mysql.TypeEnum: + canChange = true + default: + return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) + } default: if origin.Tp != to.Tp { return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) From 9b1a09501ccd74e6e47e1f519877ea8483f9bc6a Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 16 Sep 2020 14:27:41 +0800 Subject: [PATCH 03/16] modify needChangeColumn --- ddl/column.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index 17813fd704636..6ef09cf6b765f 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -644,9 +644,6 @@ func onSetDefaultValue(t *meta.Meta, job *model.Job) (ver int64, _ error) { func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { toUnsigned := mysql.HasUnsignedFlag(newCol.Flag) originUnsigned := mysql.HasUnsignedFlag(oldCol.Flag) - if newCol.Flen > 0 && newCol.Flen < oldCol.Flen || toUnsigned != originUnsigned { - return true - } if newCol.Tp == mysql.TypeEnum && oldCol.Tp == mysql.TypeEnum { if len(newCol.Elems) < len(oldCol.Elems) { return true @@ -657,6 +654,7 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { return true } } + return false } if newCol.Tp == mysql.TypeSet && oldCol.Tp == mysql.TypeSet { if len(newCol.Elems) < len(oldCol.Elems) { @@ -673,11 +671,15 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { return true } } + return false } if (newCol.Tp == mysql.TypeEnum || oldCol.Tp == mysql.TypeEnum || newCol.Tp == mysql.TypeSet || oldCol.Tp == mysql.TypeSet) && oldCol.Tp != newCol.Tp { return true } + if newCol.Flen > 0 && newCol.Flen < oldCol.Flen || toUnsigned != originUnsigned { + return true + } return false } From 5a5b7f92a8dbbba59ebd1e512ef9246b65e133a7 Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 16 Sep 2020 14:44:20 +0800 Subject: [PATCH 04/16] optimize needChangeColumnData --- ddl/column.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index 6ef09cf6b765f..81b0c42560758 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -648,9 +648,9 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { if len(newCol.Elems) < len(oldCol.Elems) { return true } - for index, originElem := range oldCol.Elems { - toElem := newCol.Elems[index] - if originElem != toElem { + for index, oldElem := range oldCol.Elems { + newElem := newCol.Elems[index] + if oldElem != newElem { return true } } @@ -665,6 +665,7 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { for _, newElem := range newCol.Elems { if oldElem == newElem { contain = true + break } } if !contain { From 34f1eddd7267dd2ec7374bbb6060d1b6381bf9dc Mon Sep 17 00:00:00 2001 From: Patrick Date: Thu, 17 Sep 2020 11:40:09 +0800 Subject: [PATCH 05/16] optimize needChangeColumnData use switch case --- ddl/column.go | 58 ++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index 81b0c42560758..77af556848e55 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -644,39 +644,45 @@ func onSetDefaultValue(t *meta.Meta, job *model.Job) (ver int64, _ error) { func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { toUnsigned := mysql.HasUnsignedFlag(newCol.Flag) originUnsigned := mysql.HasUnsignedFlag(oldCol.Flag) - if newCol.Tp == mysql.TypeEnum && oldCol.Tp == mysql.TypeEnum { - if len(newCol.Elems) < len(oldCol.Elems) { - return true - } - for index, oldElem := range oldCol.Elems { - newElem := newCol.Elems[index] - if oldElem != newElem { + switch oldCol.Tp { + case mysql.TypeEnum: + switch newCol.Tp { + case mysql.TypeEnum: + if len(newCol.Elems) < len(oldCol.Elems) { return true } - } - return false - } - if newCol.Tp == mysql.TypeSet && oldCol.Tp == mysql.TypeSet { - if len(newCol.Elems) < len(oldCol.Elems) { - return true - } - for _, oldElem := range oldCol.Elems { - contain := false - for _, newElem := range newCol.Elems { - if oldElem == newElem { - contain = true - break + for index, oldElem := range oldCol.Elems { + newElem := newCol.Elems[index] + if oldElem != newElem { + return true } } - if !contain { + return false + default: + return true + } + case mysql.TypeSet: + switch newCol.Tp { + case mysql.TypeSet: + if len(newCol.Elems) < len(oldCol.Elems) { return true } + for _, oldElem := range oldCol.Elems { + contain := false + for _, newElem := range newCol.Elems { + if oldElem == newElem { + contain = true + break + } + } + if !contain { + return true + } + } + return false + default: + return true } - return false - } - if (newCol.Tp == mysql.TypeEnum || oldCol.Tp == mysql.TypeEnum || - newCol.Tp == mysql.TypeSet || oldCol.Tp == mysql.TypeSet) && oldCol.Tp != newCol.Tp { - return true } if newCol.Flen > 0 && newCol.Flen < oldCol.Flen || toUnsigned != originUnsigned { return true From e1b96ee4373626398a5ced07e98d66b0b12dc91a Mon Sep 17 00:00:00 2001 From: Patrick Date: Tue, 22 Sep 2020 21:31:47 +0800 Subject: [PATCH 06/16] add some test --- ddl/db_test.go | 22 ++++++++++++++++++++++ expression/integration_test.go | 3 ++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index d8b705d00a8c7..41689e47e1115 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3715,6 +3715,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;") @@ -3736,6 +3737,27 @@ func (s *testSerialDBSuite) TestModifyColumnNullToNotNullWithChangingVal(c *C) { c.Assert(c2.FieldType.Tp, Equals, mysql.TypeTiny) } +func (s *testSerialDBSuite) TestModifyColumnBetweenChar(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.Se.GetSessionVars().EnableChangeColumnType = true + 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("alert table tt change a a varchar(5);") + c2 := getModifyColumn(c, s.s.(sessionctx.Context), s.schemaName, "tt", "a", false) + c.Assert(c2.FieldType.Flen, Equals, 5) + tk.MustQuery("select * from t1").Check(testkit.Rows("'111' '10000'")) + _, err := tk.Exec("alert table tt change a a varchar(4);") + c.Assert(err.Error(), Equals, "[ddl:1265]Data truncated for column 'a' at row 2") + tk.MustExec("alert table tt change a a varchar(100);") + tk.MustExec("alert table tt change a a char(10);") + c2 = getModifyColumn(c, s.s.(sessionctx.Context), s.schemaName, "tt", "a", false) + c.Assert(c2.FieldType.Tp, Equals, mysql.TypeString) + c.Assert(c2.FieldType.Flen, Equals, 10) + tk.MustQuery("select * from t1").Check(testkit.Rows("'111' '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/expression/integration_test.go b/expression/integration_test.go index d43190ed2e9c2..2b8e53b1c9c14 100755 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -7315,7 +7315,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');`, "[types:1265]Data truncated for column 'a' at row 1") } func (s *testIntegrationSerialSuite) TestIssue18949(c *C) { From 4edf90ed8ad3a21ae13bc5a532be3718e5f85736 Mon Sep 17 00:00:00 2001 From: Patrick Date: Fri, 25 Sep 2020 15:43:38 +0800 Subject: [PATCH 07/16] modify tests --- ddl/column_test.go | 2 +- ddl/db_change_test.go | 7 +++--- ddl/db_test.go | 51 +++++++++++++++++++++++++++++++++---------- 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/ddl/column_test.go b/ddl/column_test.go index bfd8f04990727..984d96c377476 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("can't change decimal column precision")}, diff --git a/ddl/db_change_test.go b/ddl/db_change_test.go index 516564d719ad7..e9bd9939f393e 100644 --- a/ddl/db_change_test.go +++ b/ddl/db_change_test.go @@ -480,12 +480,11 @@ func (s *testStateChangeSuite) TestAppendEnum(c *C) { _, 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") + _, err = s.se.Execute(context.Background(), "alter table t change c2 c2 enum('N') DEFAULT 'N'") + c.Assert(err, IsNil) 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)") + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: type int(11) not match origin enum('N')") 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_test.go b/ddl/db_test.go index 41689e47e1115..c445b5865e580 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2299,7 +2299,7 @@ func (s *testDBSuite4) TestChangeColumn(c *C) { sql = "alter table t4 change c2 a bigint not null;" tk.MustGetErrCode(sql, mysql.WarnDataTruncated) sql = "alter table t3 modify en enum('a', 'z', 'b', 'c') not null default 'a'" - tk.MustGetErrCode(sql, errno.ErrUnsupportedDDLOperation) + tk.MustExec(sql) // Rename to an existing column. s.mustExec(tk, c, "alter table t3 add column a bigint") sql = "alter table t3 change aa a bigint" @@ -3737,24 +3737,51 @@ func (s *testSerialDBSuite) TestModifyColumnNullToNotNullWithChangingVal(c *C) { c.Assert(c2.FieldType.Tp, Equals, mysql.TypeTiny) } -func (s *testSerialDBSuite) TestModifyColumnBetweenChar(c *C) { +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("alert table tt change a a varchar(5);") - c2 := getModifyColumn(c, s.s.(sessionctx.Context), s.schemaName, "tt", "a", false) + 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 t1").Check(testkit.Rows("'111' '10000'")) - _, err := tk.Exec("alert table tt change a a varchar(4);") - c.Assert(err.Error(), Equals, "[ddl:1265]Data truncated for column 'a' at row 2") - tk.MustExec("alert table tt change a a varchar(100);") - tk.MustExec("alert table tt change a a char(10);") - c2 = getModifyColumn(c, s.s.(sessionctx.Context), s.schemaName, "tt", "a", false) + 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 t1").Check(testkit.Rows("'111' '10000'")) + 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' at row 2") + 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 enum + tk.MustGetErrMsg("alter table tt change a a enum('111', '2222');", "[types:1265]Data truncated for column 'a' at row 2") + 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")) + tk.MustExec("drop table tt;") } From 1b2c383f4c27c414195e8d5b6dd876deb433245e Mon Sep 17 00:00:00 2001 From: Patrick Date: Tue, 29 Sep 2020 17:42:06 +0800 Subject: [PATCH 08/16] fix some problem --- ddl/column.go | 54 +++++++++++++------------------------------------- ddl/db_test.go | 10 ++++++++-- 2 files changed, 22 insertions(+), 42 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index fe487b53dc1ff..f60cc5c270a31 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -651,45 +651,21 @@ 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 } - switch oldCol.Tp { - case mysql.TypeEnum: - switch newCol.Tp { - case mysql.TypeEnum: - if len(newCol.Elems) < len(oldCol.Elems) { - return true - } - for index, oldElem := range oldCol.Elems { - newElem := newCol.Elems[index] - if oldElem != newElem { - return true - } - } - return false - default: + if oldCol.Tp == newCol.Tp && (oldCol.Tp == mysql.TypeEnum || oldCol.Tp == mysql.TypeSet) { + if len(newCol.Elems) < len(oldCol.Elems) { return true } - case mysql.TypeSet: - switch newCol.Tp { - case mysql.TypeSet: - if len(newCol.Elems) < len(oldCol.Elems) { + for index, oldElem := range oldCol.Elems { + newElem := newCol.Elems[index] + if oldElem != newElem { return true } - for _, oldElem := range oldCol.Elems { - contain := false - for _, newElem := range newCol.Elems { - if oldElem == newElem { - contain = true - break - } - } - if !contain { - return true - } - } - return false - default: - return true } + return false + } + 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 @@ -1136,10 +1112,8 @@ func (w *updateColumnWorker) fetchRowColVals(txn kv.Transaction, taskRange reorg taskDone := false var lastAccessedHandle kv.Handle oprStartTime := startTime - var rowIdx int err := iterateSnapshotRows(w.sessCtx.GetStore(), w.priority, w.table, txn.StartTS(), taskRange.startHandle, taskRange.endHandle, taskRange.endIncluded, func(handle kv.Handle, recordKey kv.Key, rawRow []byte) (bool, error) { - rowIdx++ oprEndTime := time.Now() logSlowOperations(oprEndTime.Sub(oprStartTime), "iterateSnapshotRows in updateColumnWorker fetchRowColVals", 0) oprStartTime = oprEndTime @@ -1155,9 +1129,6 @@ func (w *updateColumnWorker) fetchRowColVals(txn kv.Transaction, taskRange reorg } if err1 := w.getRowRecord(handle, recordKey, rawRow); err1 != nil { - if types.ErrTruncated.Equal(err1) { - err1 = types.ErrTruncated.GenWithStackByArgs(w.oldColInfo.Name, rowIdx) - } return false, errors.Trace(err1) } lastAccessedHandle = handle @@ -1192,6 +1163,9 @@ 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 { + 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]) + } if IsNormalWarning(err) || (!w.sqlMode.HasStrictMode() && IsStrictWarning(err)) { // Keep the warnings. recordWarning = errors.Cause(err).(*terror.Error) @@ -1240,7 +1214,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/db_test.go b/ddl/db_test.go index 072db4d4a797f..49764d7814134 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3907,14 +3907,20 @@ func (s *testSerialDBSuite) TestModifyColumnBetweenStringTypes(c *C) { 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' at row 2") + tk.MustGetErrMsg("alter table tt change a a set('111', '2222');", "[types:1265]Data truncated for column 'a', value is 'KindText 2222'") 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' at row 2") + tk.MustGetErrMsg("alter table tt change a a enum('111', '2222');", "[types:1265]Data truncated for column 'a', value is 'KindSet 2222'") 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) From 2978b3680528eb4cbd5a0966a01eda6c16cc765d Mon Sep 17 00:00:00 2001 From: Patrick Date: Tue, 29 Sep 2020 17:51:57 +0800 Subject: [PATCH 09/16] optimize --- ddl/column.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index f60cc5c270a31..3d47d2a6e66a8 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -652,16 +652,7 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { return oldCol.Flen != newCol.Flen || oldCol.Decimal != newCol.Decimal || toUnsigned != originUnsigned } if oldCol.Tp == newCol.Tp && (oldCol.Tp == mysql.TypeEnum || oldCol.Tp == mysql.TypeSet) { - if len(newCol.Elems) < len(oldCol.Elems) { - return true - } - for index, oldElem := range oldCol.Elems { - newElem := newCol.Elems[index] - if oldElem != newElem { - return true - } - } - return false + return isElemsChangedToModifyColumn(oldCol.Elems, newCol.Elems) } if oldCol.Tp == mysql.TypeEnum || oldCol.Tp == mysql.TypeSet || newCol.Tp == mysql.TypeEnum || newCol.Tp == mysql.TypeSet { @@ -673,6 +664,19 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { 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 +} + type modifyColumnJobParameter struct { newCol *model.ColumnInfo oldColName *model.CIStr From 8431f0df51f9d9e0a9797860a6efb19af2bf20aa Mon Sep 17 00:00:00 2001 From: Patrick Date: Tue, 29 Sep 2020 19:09:31 +0800 Subject: [PATCH 10/16] add isErrorToRollback --- ddl/column.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index 3d47d2a6e66a8..f4b37be793053 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -919,8 +919,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) || - types.ErrOverflow.Equal(err) || types.ErrDataTooLong.Equal(err) || types.ErrTruncated.Equal(err) { + if isErrorToRollback(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)) @@ -979,6 +978,12 @@ func (w *worker) doModifyColumnTypeWithData( return ver, errors.Trace(err) } +// isErrorToRollback some error need roll back data +func isErrorToRollback(err error) bool { + return kv.ErrKeyExists.Equal(err) || errCancelledDDLJob.Equal(err) || errCantDecodeRecord.Equal(err) || types.ErrOverflow.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) From c759b888867c63e58a2fc57f9d8085e1af44c1d2 Mon Sep 17 00:00:00 2001 From: Patrick Date: Thu, 1 Oct 2020 14:27:58 +0800 Subject: [PATCH 11/16] modify tests --- ddl/db_test.go | 4 ++-- expression/integration_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 49764d7814134..49d6e3c9552ac 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3907,7 +3907,7 @@ func (s *testSerialDBSuite) TestModifyColumnBetweenStringTypes(c *C) { 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 'KindText 2222'") + 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) @@ -3920,7 +3920,7 @@ func (s *testSerialDBSuite) TestModifyColumnBetweenStringTypes(c *C) { 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 'KindSet 2222'") + tk.MustGetErrMsg("alter table tt change a a enum('111', '2222');", "[types:1265]Data truncated for column 'a', value is 'KindSet 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) diff --git a/expression/integration_test.go b/expression/integration_test.go index c529f82ff941a..43da238f7006b 100755 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -7330,7 +7330,7 @@ func (s *testIntegrationSerialSuite) TestIssue19804(c *C) { tk.MustExec(`create table t(a set('a', 'b', 'c'));`) tk.MustExec(`alter table t change a a set('a', 'b', 'c', 'd');`) tk.MustExec(`insert into t values('d');`) - tk.MustGetErrMsg(`alter table t change a a set('a', 'b', 'c', 'e', 'f');`, "[types:1265]Data truncated for column 'a' at row 1") + tk.MustGetErrMsg(`alter table t change a a set('a', 'b', 'c', 'e', 'f');`, "[types:1265]Data truncated for column 'a', value is 'KindMysqlSet d'") } func (s *testIntegrationSerialSuite) TestIssue18949(c *C) { From ce0263f38873c382d28c3ca09b1aa2131e23be90 Mon Sep 17 00:00:00 2001 From: Patrick Date: Thu, 1 Oct 2020 14:34:48 +0800 Subject: [PATCH 12/16] modify tests --- ddl/db_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 49d6e3c9552ac..b7447ec529ea9 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3920,7 +3920,7 @@ func (s *testSerialDBSuite) TestModifyColumnBetweenStringTypes(c *C) { 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 'KindSet 10000'") + 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) From 35606375a09e51c2888a8f64c5a1cc9e1922a3bc Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 14 Oct 2020 15:58:25 +0800 Subject: [PATCH 13/16] fix review problems --- ddl/column.go | 19 +++++++++++++------ ddl/db_test.go | 5 +++++ ddl/ddl_api.go | 22 ++-------------------- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index 6a3de4204cfff..31af364979c6e 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -920,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 isErrorToRollback(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)) @@ -979,8 +979,8 @@ func (w *worker) doModifyColumnTypeWithData( return ver, errors.Trace(err) } -// isErrorToRollback some error need roll back data -func isErrorToRollback(err error) bool { +// 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.ErrOverflow.Equal(err) || types.ErrDataTooLong.Equal(err) || types.ErrTruncated.Equal(err) } @@ -1173,9 +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 { - 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]) - } + err = w.reformatErrors(err) if IsNormalWarning(err) || (!w.sqlMode.HasStrictMode() && IsStrictWarning(err)) { // Keep the warnings. recordWarning = errors.Cause(err).(*terror.Error) @@ -1210,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 { diff --git a/ddl/db_test.go b/ddl/db_test.go index b7447ec529ea9..5d700c6a79d8b 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3929,6 +3929,11 @@ func (s *testSerialDBSuite) TestModifyColumnBetweenStringTypes(c *C) { 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;") } diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index cf50583b2f453..45c7875a13a71 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3294,8 +3294,8 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al unsupportedMsg := fmt.Sprintf("type %v not match origin %v", to.CompactStr(), origin.CompactStr()) 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, mysql.TypeEnum, mysql.TypeSet: switch to.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, @@ -3311,15 +3311,6 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al default: return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } - case mysql.TypeEnum: - switch to.Tp { - case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, - mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, - mysql.TypeSet, mysql.TypeEnum: - canChange = true - default: - return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) - } case mysql.TypeNewDecimal: if origin.Tp != to.Tp { return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) @@ -3331,15 +3322,6 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al msg := fmt.Sprintf("decimal change from decimal(%d, %d) to decimal(%d, %d)", origin.Flen, origin.Decimal, to.Flen, to.Decimal) return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) } - case mysql.TypeSet: - switch to.Tp { - case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, - mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, - mysql.TypeSet, mysql.TypeEnum: - canChange = true - default: - return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) - } default: if origin.Tp != to.Tp { return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) From 0e92d60f68c4d95ee74644f1750681d2f68d448a Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 14 Oct 2020 19:20:33 +0800 Subject: [PATCH 14/16] fix ErrOverflow error duplicate --- ddl/column.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/column.go b/ddl/column.go index 31af364979c6e..e4b7dd0a6815a 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -981,7 +981,7 @@ func (w *worker) doModifyColumnTypeWithData( // 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) || + return kv.ErrKeyExists.Equal(err) || errCancelledDDLJob.Equal(err) || errCantDecodeRecord.Equal(err) || types.ErrOverflow.Equal(err) || types.ErrDataTooLong.Equal(err) || types.ErrTruncated.Equal(err) } From a4f1a029d92033e6b78061c91e974d68f2211f2d Mon Sep 17 00:00:00 2001 From: Patrick Date: Thu, 15 Oct 2020 19:56:02 +0800 Subject: [PATCH 15/16] fix TypeEnum convert compatible with old version --- ddl/db_change_test.go | 6 +++--- ddl/db_test.go | 2 +- ddl/ddl_api.go | 23 ++++++++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/ddl/db_change_test.go b/ddl/db_change_test.go index e9bd9939f393e..92a2514ff1c26 100644 --- a/ddl/db_change_test.go +++ b/ddl/db_change_test.go @@ -479,9 +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") - _, err = s.se.Execute(context.Background(), "alter table t change c2 c2 enum('N') DEFAULT 'N'") - c.Assert(err, IsNil) + 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, 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: type int(11) not match origin enum('N')") diff --git a/ddl/db_test.go b/ddl/db_test.go index 5d700c6a79d8b..5b395ebf760f4 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2323,7 +2323,7 @@ func (s *testDBSuite4) TestChangeColumn(c *C) { sql = "alter table t4 change c2 a bigint not null;" tk.MustGetErrCode(sql, mysql.WarnDataTruncated) sql = "alter table t3 modify en enum('a', 'z', 'b', 'c') not null default 'a'" - tk.MustExec(sql) + tk.MustGetErrCode(sql, errno.ErrUnsupportedDDLOperation) // Rename to an existing column. s.mustExec(tk, c, "alter table t3 add column a bigint") sql = "alter table t3 change aa a bigint" diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 45c7875a13a71..6a836a4619b6b 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3295,7 +3295,7 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al var canChange bool switch origin.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, - mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, mysql.TypeEnum, mysql.TypeSet: + mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, mysql.TypeSet: switch to.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, @@ -3322,6 +3322,27 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al msg := fmt.Sprintf("decimal change from decimal(%d, %d) to decimal(%d, %d)", origin.Flen, origin.Decimal, to.Flen, to.Decimal) return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) } + case mysql.TypeEnum: + switch to.Tp { + case mysql.TypeEnum: + 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 msg, 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 msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } + } + case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, + mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, + mysql.TypeSet: + canChange = true + default: + return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) + } default: if origin.Tp != to.Tp { return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) From 3e7a9920c5c585442de3f6c6bacd5f1222cd53b0 Mon Sep 17 00:00:00 2001 From: Patrick Date: Thu, 15 Oct 2020 20:55:18 +0800 Subject: [PATCH 16/16] fix convert compatible with old version --- ddl/db_change_test.go | 2 +- ddl/ddl_api.go | 48 ++++++++++++++++++++-------------- expression/integration_test.go | 2 +- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/ddl/db_change_test.go b/ddl/db_change_test.go index 92a2514ff1c26..1e34778dadea8 100644 --- a/ddl/db_change_test.go +++ b/ddl/db_change_test.go @@ -484,7 +484,7 @@ func (s *testStateChangeSuite) TestAppendEnum(c *C) { 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: type int(11) not match origin enum('N')") + c.Assert(err.Error(), Equals, "[ddl:8200]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/ddl_api.go b/ddl/ddl_api.go index 6a287a74cac81..7330d69ca9917 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3295,12 +3295,13 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al var canChange bool switch origin.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, mysql.TypeBlob, - mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, mysql.TypeSet: + mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: switch to.Tp { case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString, - mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, - mysql.TypeEnum, mysql.TypeSet: + 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) } @@ -3311,38 +3312,45 @@ func CheckModifyTypeCompatible(origin *types.FieldType, to *types.FieldType) (al default: return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } - case mysql.TypeNewDecimal: - if origin.Tp != to.Tp { - return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) - } - // Floating-point and fixed-point types also can be UNSIGNED. As with integer types, this attribute prevents - // negative values from being stored in the column. Unlike the integer types, the upper range of column values - // remains the same. - if to.Flen != origin.Flen || to.Decimal != origin.Decimal || mysql.HasUnsignedFlag(to.Flag) != mysql.HasUnsignedFlag(origin.Flag) { - msg := fmt.Sprintf("decimal change from decimal(%d, %d) to decimal(%d, %d)", origin.Flen, origin.Decimal, to.Flen, to.Decimal) - return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) + case mysql.TypeEnum, mysql.TypeSet: + var typeVar string + if origin.Tp == mysql.TypeEnum { + typeVar = "enum" + } else { + typeVar = "set" } - case mysql.TypeEnum: switch to.Tp { - case mysql.TypeEnum: + case mysql.TypeEnum, mysql.TypeSet: 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)) + 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 enum column value %s to %s", 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, - mysql.TypeSet: - canChange = true + 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) + } + case mysql.TypeNewDecimal: + if origin.Tp != to.Tp { return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) } + // Floating-point and fixed-point types also can be UNSIGNED. As with integer types, this attribute prevents + // negative values from being stored in the column. Unlike the integer types, the upper range of column values + // remains the same. + if to.Flen != origin.Flen || to.Decimal != origin.Decimal || mysql.HasUnsignedFlag(to.Flag) != mysql.HasUnsignedFlag(origin.Flag) { + msg := fmt.Sprintf("decimal change from decimal(%d, %d) to decimal(%d, %d)", origin.Flen, origin.Decimal, to.Flen, to.Decimal) + return msg, errUnsupportedModifyColumn.GenWithStackByArgs(msg) + } default: if origin.Tp != to.Tp { return "", errUnsupportedModifyColumn.GenWithStackByArgs(unsupportedMsg) diff --git a/expression/integration_test.go b/expression/integration_test.go index f7e9b2ebac893..3027557fb16e8 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -7483,7 +7483,7 @@ func (s *testIntegrationSerialSuite) TestIssue19804(c *C) { tk.MustExec(`create table t(a set('a', 'b', 'c'));`) tk.MustExec(`alter table t change a a set('a', 'b', 'c', 'd');`) tk.MustExec(`insert into t values('d');`) - tk.MustGetErrMsg(`alter table t change a a set('a', 'b', 'c', 'e', 'f');`, "[types:1265]Data truncated for column 'a', value is 'KindMysqlSet 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) {