From 11bc9c139dbd4a73f20859142f97ac7cbca5ae58 Mon Sep 17 00:00:00 2001 From: Lenshood <7877221+LENSHOOD@users.noreply.github.com> Date: Thu, 22 Jul 2021 16:06:13 +0800 Subject: [PATCH] ddl: do change column data when binary to binary with different length (#24681) --- ddl/column.go | 11 ++++++++--- ddl/db_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++---- ddl/ddl_api.go | 12 ++++++++++-- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index d50f2618d6..bc44ed8529 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -691,10 +691,10 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) 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) + defaultOldColFlen, _ := mysql.GetDefaultFieldLengthAndDecimal(oldCol.Tp) + defaultNewColFlen, _ := mysql.GetDefaultFieldLengthAndDecimal(newCol.Tp) needTruncationOrToggleSignForInteger := func() bool { - return (newColFlen > 0 && newColFlen < oldColFlen) || (toUnsigned != originUnsigned) + return (defaultNewColFlen > 0 && defaultNewColFlen < defaultOldColFlen) || (toUnsigned != originUnsigned) } // Deal with the same type. @@ -708,6 +708,11 @@ func needChangeColumnData(oldCol, newCol *model.ColumnInfo) bool { return isElemsChangedToModifyColumn(oldCol.Elems, newCol.Elems) case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong: return toUnsigned != originUnsigned + case mysql.TypeString: + // Due to the behavior of padding \x00 at binary type, always change column data when binary length changed + if types.IsBinaryStr(&oldCol.FieldType) { + return newCol.Flen != oldCol.Flen + } } return needTruncationOrToggleSign() diff --git a/ddl/db_test.go b/ddl/db_test.go index 01c2546be6..bf123ddb91 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -4060,7 +4060,7 @@ func (s *testDBSuite5) TestModifyColumnRollBack(c *C) { s.mustExec(tk, c, "drop table t1") } -func (s *testSerialDBSuite) TestModifyColumnnReorgInfo(c *C) { +func (s *testSerialDBSuite) TestModifyColumnReorgInfo(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test_db") tk.MustExec("drop table if exists t1") @@ -4210,15 +4210,58 @@ func (s *testSerialDBSuite) TestModifyColumnBetweenStringTypes(c *C) { 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) + mvc := getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(mvc.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);") + tk.MustQuery("select length(a) from tt").Check(testkit.Rows("3", "5")) + + // char to char + tk.MustExec("drop table if exists tt;") + tk.MustExec("create table tt (a char(10));") + tk.MustExec("insert into tt values ('111'),('10000');") + tk.MustExec("alter table tt change a a char(5);") + mc := getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(mc.FieldType.Flen, Equals, 5) + 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") + tk.MustExec("alter table tt change a a char(100);") + tk.MustQuery("select length(a) from tt").Check(testkit.Rows("3", "5")) + + // binary to binary + tk.MustExec("drop table if exists tt;") + tk.MustExec("create table tt (a binary(10));") + tk.MustExec("insert into tt values ('111'),('10000');") + tk.MustGetErrMsg("alter table tt change a a binary(5);", "[types:1406]Data Too Long, field len 5, data len 10") + mb := getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(mb.FieldType.Flen, Equals, 10) + tk.MustQuery("select * from tt").Check(testkit.Rows("111\x00\x00\x00\x00\x00\x00\x00", "10000\x00\x00\x00\x00\x00")) + tk.MustGetErrMsg("alter table tt change a a binary(4);", "[types:1406]Data Too Long, field len 4, data len 10") + tk.MustExec("alter table tt change a a binary(12);") + tk.MustQuery("select * from tt").Check(testkit.Rows("111\x00\x00\x00\x00\x00\x00\x00\x00\x00", "10000\x00\x00\x00\x00\x00\x00\x00")) + tk.MustQuery("select length(a) from tt").Check(testkit.Rows("12", "12")) + + // varbinary to varbinary + tk.MustExec("drop table if exists tt;") + tk.MustExec("create table tt (a varbinary(10));") + tk.MustExec("insert into tt values ('111'),('10000');") + tk.MustExec("alter table tt change a a varbinary(5);") + mvb := getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + c.Assert(mvb.FieldType.Flen, Equals, 5) + tk.MustQuery("select * from tt").Check(testkit.Rows("111", "10000")) + tk.MustGetErrMsg("alter table tt change a a varbinary(4);", "[types:1406]Data Too Long, field len 4, data len 5") + tk.MustExec("alter table tt change a a varbinary(12);") + tk.MustQuery("select * from tt").Check(testkit.Rows("111", "10000")) + tk.MustQuery("select length(a) from tt").Check(testkit.Rows("3", "5")) // varchar to char + 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 char(10);") - c2 = getModifyColumn(c, s.s.(sessionctx.Context), "test", "tt", "a", false) + 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")) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 7f0e0b0488..faa026115e 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -3575,8 +3575,16 @@ func needReorgToChange(origin *types.FieldType, to *types.FieldType) (needReorg return true, "conversion between char and varchar string needs reorganization" } - if toFlen > 0 && toFlen < originFlen { - return true, fmt.Sprintf("length %d is less than origin %d", toFlen, originFlen) + if toFlen > 0 && toFlen != originFlen { + if toFlen < originFlen { + return true, fmt.Sprintf("length %d is less than origin %d", toFlen, originFlen) + } + + // Due to the behavior of padding \x00 at binary type, we need to reorg when binary length changed + isBinaryType := func(tp *types.FieldType) bool { return tp.Tp == mysql.TypeString && types.IsBinaryStr(tp) } + if isBinaryType(origin) && isBinaryType(to) { + return true, "can't change binary types of different length" + } } if to.Decimal > 0 && to.Decimal < origin.Decimal { return true, fmt.Sprintf("decimal %d is less than origin %d", to.Decimal, origin.Decimal)