ddl: support modify decimal column precision in some cases.

ddl/column_test.go

@@ -937,27 +937,43 @@ func (s *testColumnSuite) TestModifyColumn(c *C) {
 		WithLease(testLease),
 	)
 	defer d.Stop()
+
+	tblInfo := testTableInfo(c, d, "t", 2)
+	idx := &model.IndexInfo{
+		ID:   1,
+		Name: model.NewCIStr("idx"),
+		Columns: []*model.IndexColumn{
+			{
+				Name: tblInfo.Columns[0].Name, // column[0] has index covered.
+			},
+		},
+	}
+	tblInfo.Indices = append(tblInfo.Indices, idx)
 	tests := []struct {
 		origin string
 		to     string
 		err    error
+		col    *model.ColumnInfo
 	}{
-		{"int", "bigint", nil},
-		{"int", "int unsigned", errUnsupportedModifyColumn.GenWithStackByArgs("length 10 is less than origin 11")},
-		{"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(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")},
-		{"decimal(2,1)", "decimal(2,2)", errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision")},
-		{"decimal(2,1)", "decimal(2,1)", nil},
+		{"int", "bigint", nil, tblInfo.Columns[0]},
+		{"int", "int unsigned", errUnsupportedModifyColumn.GenWithStackByArgs("length 10 is less than origin 11"), tblInfo.Columns[0]},
+		{"varchar(10)", "text", nil, tblInfo.Columns[0]},
+		{"varbinary(10)", "blob", nil, tblInfo.Columns[0]},
+		{"text", "blob", errUnsupportedModifyCharset.GenWithStackByArgs("charset from utf8mb4 to binary"), tblInfo.Columns[0]},
+		{"varchar(10)", "varchar(8)", errUnsupportedModifyColumn.GenWithStackByArgs("length 8 is less than origin 10"), tblInfo.Columns[0]},
+		{"varchar(10)", "varchar(11)", nil, tblInfo.Columns[0]},
+		{"varchar(10) character set utf8 collate utf8_bin", "varchar(10) character set utf8", nil, tblInfo.Columns[0]},
+		{"decimal(2,1)", "decimal(3,2)", errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision with index covered now"), tblInfo.Columns[0]},
+		{"decimal(2,1)", "decimal(2,2)", errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision with index covered now"), tblInfo.Columns[0]},
+		{"decimal(2,1)", "decimal(2,2)", errUnsupportedModifyColumn.GenWithStackByArgs("modify original decimal(2,1) to decimal(2,2) may cause out of range value error"), tblInfo.Columns[1]},
+		{"decimal(2,1)", "decimal(2,1)", nil, tblInfo.Columns[0]},
+		{"decimal(2,1)", "decimal(3,1)", nil, tblInfo.Columns[1]}, // column[1] doesn't have index covered.
 	}
 	for _, tt := range tests {
 		ftA := s.colDefStrToFieldType(c, tt.origin)
 		ftB := s.colDefStrToFieldType(c, tt.to)
-		err := modifiable(ftA, ftB)
+		tt.col.FieldType = *ftA
+		err := modifiable(tblInfo, tt.col, ftB)
 		if err == nil {
 			c.Assert(tt.err, IsNil)
 		} else {

ddl/db_test.go

@@ -3683,7 +3683,72 @@ func (s *testDBSuite1) TestModifyColumnCharset(c *C) {
 			"  `a` varchar(8) DEFAULT NULL,\n" +
 			"  `b` varchar(8) CHARACTER SET utf8 COLLATE utf8_bin DEFAULT NULL\n" +
 			") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin"))
+}
 
+func (s *testDBSuite1) TestModifyColumnWithDecimal(c *C) {
+	s.tk = testkit.NewTestKit(c, s.store)
+	s.tk.MustExec("use test_db;")
+	s.tk.MustExec("create table t_decimal (a int,b decimal(10,5),c varchar(10), index idx1(b),index idx2(a,b,c),index idx3(a));")
+	s.tk.MustExec("insert into t_decimal values (1,12345.67891,'a'),(2,123.456,'b'),(4,0.0,'d');")
+	// Test modify decimal column with index covered.
+	s.tk.MustExec("alter table t_decimal modify column b decimal(10,5);")
+	testExecErrorMessage(c, s.tk, "alter table t_decimal modify column b decimal(10,6);",
+		"[ddl:8200]Unsupported modify column: can't change decimal column precision with index covered now")
+	testExecErrorMessage(c, s.tk, "alter table t_decimal modify column b decimal(11,5);",
+		"[ddl:8200]Unsupported modify column: can't change decimal column precision with index covered now")
+	testExecErrorMessage(c, s.tk, "alter table t_decimal modify column b decimal(11,6);",
+		"[ddl:8200]Unsupported modify column: can't change decimal column precision with index covered now")
+	s.tk.MustExec("alter table t_decimal drop index idx1;")
+	testExecErrorMessage(c, s.tk, "alter table t_decimal modify column b decimal(11,6);",
+		"[ddl:8200]Unsupported modify column: can't change decimal column precision with index covered now")
+	s.tk.MustExec("alter table t_decimal drop index idx2;")
+	testExecErrorMessage(c, s.tk, "alter table t_decimal modify column b decimal(9,6);",
+		"[ddl:8200]Unsupported modify column: length 9 is less than origin 10")
+
+	// Test modify decimal column that only enlarge decimal size may cause out of range value error.
+	testExecErrorMessage(c, s.tk, "alter table t_decimal modify column b decimal(10,6);",
+		"[ddl:8200]Unsupported modify column: modify original decimal(10,5) to decimal(10,6) may cause out of range value error")
+
+	// Test modify column successful when no index covered.
+	s.tk.MustExec("alter table t_decimal modify column b decimal(11,6);")
+	s.tk.MustExec("insert into t_decimal values (3, 12345.678912, 'c');")
+	s.tk.MustQuery("select * from t_decimal order by a").Check(testkit.Rows("1 12345.678910 a", "2 123.456000 b", "3 12345.678912 c", "4 0.000000 d"))
+	s.tk.MustExec("alter table t_decimal add index idx4(b);")
+	s.tk.MustQuery("select * from t_decimal use index(idx4) where b=12345.67891").Check(testkit.Rows("1 12345.678910 a"))
+	s.tk.MustQuery("select * from t_decimal use index(idx4) where b=123.456").Check(testkit.Rows("2 123.456000 b"))
+	s.tk.MustQuery("select * from t_decimal use index(idx4) where b=12345.678912").Check(testkit.Rows("3 12345.678912 c"))
+	s.tk.MustQuery("select * from t_decimal use index(idx4) where b=0").Check(testkit.Rows("4 0.000000 d"))
+	s.tk.MustQuery("select b from t_decimal use index(idx4) where b=12345.67891").Check(testkit.Rows("12345.678910"))
+	s.tk.MustQuery("select b from t_decimal use index(idx4) where b=123.456").Check(testkit.Rows("123.456000"))
+	s.tk.MustQuery("select b from t_decimal use index(idx4) where b=12345.678912").Check(testkit.Rows("12345.678912"))
+	s.tk.MustQuery("select b from t_decimal use index(idx4) where b=0").Check(testkit.Rows("0.000000"))
+	s.tk.MustExec("alter table t_decimal add index idx5(a,b,c);")
+
+	// Test update with decimal.
+	s.tk.MustExec("update t_decimal set a=a+5 where a=1;")
+	s.tk.MustQuery("select * from t_decimal use index(idx4) where b=12345.67891").Check(testkit.Rows("6 12345.678910 a"))
+	s.tk.MustQuery("select b from t_decimal use index(idx4) where b=12345.67891").Check(testkit.Rows("12345.678910"))
+	s.tk.MustExec("update t_decimal set b=b+1 where a=6;")
+	s.tk.MustQuery("select * from t_decimal use index(idx4) where b=12346.67891").Check(testkit.Rows("6 12346.678910 a"))
+	s.tk.MustQuery("select b from t_decimal use index(idx4) where b=12346.67891").Check(testkit.Rows("12346.678910"))
+
+	// Test for add unique index and insert.
+	s.tk.MustExec("insert into t_decimal values (7,123.456,null);")
+	s.tk.MustQuery("select * from t_decimal use index(idx4) where b=123.456 order by b").Check(testkit.Rows("2 123.456000 b", "7 123.456000 <nil>"))
+	testExecErrorMessage(c, s.tk, "alter table t_decimal add unique index idx6(b);",
+		"[kv:1062]Duplicate entry '' for key 'idx6'")
+	s.tk.MustExec("delete from t_decimal where b=123.456")
+	s.tk.MustExec("alter table t_decimal add unique index idx6(b);")
+	s.tk.MustExec("insert into t_decimal values (8,123.456,'bbb');")
+	testExecErrorMessage(c, s.tk, "insert into t_decimal values (9,123.456,'bbb');",
+		"[kv:1062]Duplicate entry '123.456000' for key 'idx6'")
+	s.tk.MustExec("admin check table t_decimal;")
+}
+
+func testExecErrorMessage(c *C, tk *testkit.TestKit, sql, errMsg string) {
+	_, err := tk.Exec(sql)
+	c.Assert(err, NotNil)
+	c.Assert(err.Error(), Equals, errMsg)
 }
 
 func (s *testDBSuite1) TestSetTableFlashReplica(c *C) {

ddl/ddl_api.go

@@ -2531,7 +2531,8 @@ func modifiableCharsetAndCollation(toCharset, toCollate, origCharset, origCollat
 // change or check existing data in the table.
 // It returns true if the two types has the same Charset and Collation, the same sign, both are
 // integer types or string types, and new Flen and Decimal must be greater than or equal to origin.
-func modifiable(origin *types.FieldType, to *types.FieldType) error {
+func modifiable(tbInfo *model.TableInfo, originalCol *model.ColumnInfo, to *types.FieldType) error {
+	origin := &originalCol.FieldType
 	unsupportedMsg := fmt.Sprintf("type %v not match origin %v", to.CompactStr(), origin.CompactStr())
 	switch origin.Tp {
 	case mysql.TypeVarchar, mysql.TypeString, mysql.TypeVarString,
@@ -2565,9 +2566,21 @@ func modifiable(origin *types.FieldType, to *types.FieldType) error {
 			}
 		}
 	case mysql.TypeNewDecimal:
-		// The root cause is modifying decimal precision needs to rewrite binary representation of that decimal.
 		if to.Flen != origin.Flen || to.Decimal != origin.Decimal {
-			return errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision")
+			for _, indexInfo := range tbInfo.Indices {
+				containColumn := false
+				for _, col := range indexInfo.Columns {
+					if col.Name.L == originalCol.Name.L {
+						containColumn = true
+						break
+					}
+				}
+				if containColumn {
+					// The root cause is modifying decimal precision needs to rewrite binary representation of that decimal.
+					// If has index cover this column, need to re-create the index to avoid data inconsistency.
+					return errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision with index covered now")
+				}
+			}
 		}
 	default:
 		if origin.Tp != to.Tp {
@@ -2583,6 +2596,11 @@ func modifiable(origin *types.FieldType, to *types.FieldType) error {
 		msg := fmt.Sprintf("decimal %d is less than origin %d", to.Decimal, origin.Decimal)
 		return errUnsupportedModifyColumn.GenWithStackByArgs(msg)
 	}
+	if (to.Flen - to.Decimal) < (origin.Flen - origin.Decimal) {
+		msg := fmt.Sprintf("modify original decimal(%d,%d) to decimal(%d,%d) may cause out of range value error",
+			origin.Flen, origin.Decimal, to.Flen, to.Decimal)
+		return errUnsupportedModifyColumn.GenWithStackByArgs(msg)
+	}
 
 	toUnsigned := mysql.HasUnsignedFlag(to.Flag)
 	originUnsigned := mysql.HasUnsignedFlag(origin.Flag)
@@ -2786,7 +2804,7 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or
 		return nil, errors.Trace(err)
 	}
 
-	if err = modifiable(&col.FieldType, &newCol.FieldType); err != nil {
+	if err = modifiable(t.Meta(), col.ColumnInfo, &newCol.FieldType); err != nil {
 		return nil, errors.Trace(err)
 	}
 

executor/admin_test.go

@@ -716,7 +716,7 @@ func (s *testSuite2) TestAdminCheckTable(c *C) {
 	tk.MustExec(`insert into t1 set a='1.9'`)
 	err = tk.ExecToErr(`alter table t1 modify column a decimal(3,2);`)
 	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: can't change decimal column precision")
+	c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported modify column: can't change decimal column precision with index covered now")
 	tk.MustExec(`delete from t1;`)
 	tk.MustExec(`admin check table t1;`)
 }

executor/distsql.go

@@ -873,7 +873,10 @@ func (w *tableWorker) compareData(ctx context.Context, task *lookupTableTask, ta
 					vals = append(vals, row.GetDatum(i, &col.FieldType))
 				}
 			}
-			vals = tables.TruncateIndexValuesIfNeeded(tblInfo, w.idxLookup.index, vals)
+			vals, err = tables.FormatIndexValuesIfNeeded(tblReaderExec.ctx.GetSessionVars().StmtCtx, tblInfo, w.idxLookup.index, vals)
+			if err != nil {
+				return errors.Trace(err)
+			}
 			for i, val := range vals {
 				col := w.idxTblCols[i]
 				tp := &col.FieldType

table/tables/index.go

@@ -124,11 +124,14 @@ func (c *index) getIndexKeyBuf(buf []byte, defaultCap int) []byte {
 	return make([]byte, 0, defaultCap)
 }
 
-// TruncateIndexValuesIfNeeded truncates the index values created using only the leading part of column values.
-func TruncateIndexValuesIfNeeded(tblInfo *model.TableInfo, idxInfo *model.IndexInfo, indexedValues []types.Datum) []types.Datum {
+// FormatIndexValuesIfNeeded formats the index value if needed.
+// For string column, truncates the index values created using only the leading part of column values.
+// For decimal column, convert to the column info precision.
+func FormatIndexValuesIfNeeded(sc *stmtctx.StatementContext, tblInfo *model.TableInfo, idxInfo *model.IndexInfo, indexedValues []types.Datum) ([]types.Datum, error) {
 	for i := 0; i < len(indexedValues); i++ {
 		v := &indexedValues[i]
-		if v.Kind() == types.KindString || v.Kind() == types.KindBytes {
+		switch v.Kind() {
+		case types.KindString, types.KindBytes:
 			ic := idxInfo.Columns[i]
 			colCharset := tblInfo.Columns[ic.Offset].Charset
 			colValue := v.GetBytes()
@@ -151,10 +154,18 @@ func TruncateIndexValuesIfNeeded(tblInfo *model.TableInfo, idxInfo *model.IndexI
 					v.SetString(v.GetString())
 				}
 			}
+		case types.KindMysqlDecimal:
+			col := tblInfo.Columns[idxInfo.Columns[i].Offset]
+			// Always use the column precision.
+			value, err := v.ConvertTo(sc, &col.FieldType)
+			if err != nil {
+				return nil, err
+			}
+			indexedValues[i] = value
 		}
 	}
 
-	return indexedValues
+	return indexedValues, nil
 }
 
 // GenIndexKey generates storage key for index values. Returned distinct indicates whether the
@@ -176,7 +187,11 @@ func (c *index) GenIndexKey(sc *stmtctx.StatementContext, indexedValues []types.
 
 	// For string columns, indexes can be created using only the leading part of column values,
 	// using col_name(length) syntax to specify an index prefix length.
-	indexedValues = TruncateIndexValuesIfNeeded(c.tblInfo, c.idxInfo, indexedValues)
+	// For decimal column, convert to the column info precision.
+	indexedValues, err = FormatIndexValuesIfNeeded(sc, c.tblInfo, c.idxInfo, indexedValues)
+	if err != nil {
+		return nil, false, err
+	}
 	key = c.getIndexKeyBuf(buf, len(c.prefix)+len(indexedValues)*9+9)
 	key = append(key, []byte(c.prefix)...)
 	key, err = codec.EncodeKey(sc, key, indexedValues...)

util/codec/codec.go

@@ -989,7 +989,7 @@ func (decoder *Decoder) DecodeOne(b []byte, colIdx int, ft *types.FieldType) (re
 		if err != nil {
 			return nil, errors.Trace(err)
 		}
-		if ft.Decimal != types.UnspecifiedLength && frac > ft.Decimal {
+		if ft.Decimal != types.UnspecifiedLength && frac != ft.Decimal {
 			to := new(types.MyDecimal)
 			err := dec.Round(to, ft.Decimal, types.ModeHalfEven)
 			if err != nil {