From f8b713a9436a8ab62c79c4338c31af8b3f06a960 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 6 Jan 2020 16:09:13 +0800 Subject: [PATCH 01/11] ddl: support modify decimal column when no index covered --- ddl/column_test.go | 39 ++++++++++++++++++--------- ddl/db_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++ ddl/ddl_api.go | 23 ++++++++++++---- executor/admin_test.go | 2 +- executor/distsql.go | 5 +++- table/tables/index.go | 20 +++++++++++--- util/codec/codec.go | 2 +- 7 files changed, 128 insertions(+), 24 deletions(-) diff --git a/ddl/column_test.go b/ddl/column_test.go index 16f682543950e..4c4802e79bf0f 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -937,27 +937,42 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { WithLease(testLease), ) defer d.Stop() + + tblInfo := testTableInfo(c, d, "t", 4) + idx := &model.IndexInfo{ + ID: 1, + Name: model.NewCIStr("idx"), + Columns: []*model.IndexColumn{ + { + Name: tblInfo.Columns[0].Name, + }, + }, + } + 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)", nil, tblInfo.Columns[1]}, + {"decimal(2,1)", "decimal(2,1)", nil, tblInfo.Columns[0]}, } 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 { diff --git a/ddl/db_test.go b/ddl/db_test.go index 44e254376c155..6179e5a345f03 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3668,7 +3668,68 @@ 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 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 ")) + 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) { diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 644ba98ab29aa..d0ca2e240f002 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2524,7 +2524,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, @@ -2558,9 +2559,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 to.Flen != origin.Flen || to.Decimal != origin.Decimal { + return errUnsupportedModifyColumn.GenWithStackByArgs("can't change decimal column precision with index covered now") + } + break + } } default: if origin.Tp != to.Tp { @@ -2779,7 +2792,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) } diff --git a/executor/admin_test.go b/executor/admin_test.go index 93e0a192d6bcf..8be3406edd4d6 100644 --- a/executor/admin_test.go +++ b/executor/admin_test.go @@ -714,7 +714,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;`) } diff --git a/executor/distsql.go b/executor/distsql.go index c824d526ee4b9..385ac489dae00 100644 --- a/executor/distsql.go +++ b/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.TruncateIndexValuesIfNeeded(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 diff --git a/table/tables/index.go b/table/tables/index.go index b77422b2ec505..8fb3a9f80256c 100644 --- a/table/tables/index.go +++ b/table/tables/index.go @@ -125,10 +125,11 @@ func (c *index) getIndexKeyBuf(buf []byte, defaultCap int) []byte { } // 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 { +func TruncateIndexValuesIfNeeded(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 +152,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 +185,10 @@ 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) + indexedValues, err = TruncateIndexValuesIfNeeded(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...) diff --git a/util/codec/codec.go b/util/codec/codec.go index c5c11e12a89cb..d7bf3b934a9c9 100644 --- a/util/codec/codec.go +++ b/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 { From 914b520f735be95ab5d12314de08dc615230ca67 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 6 Jan 2020 16:54:55 +0800 Subject: [PATCH 02/11] refine comment --- executor/distsql.go | 2 +- table/tables/index.go | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index 385ac489dae00..c222d9d976fd3 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -873,7 +873,7 @@ func (w *tableWorker) compareData(ctx context.Context, task *lookupTableTask, ta vals = append(vals, row.GetDatum(i, &col.FieldType)) } } - vals, err = tables.TruncateIndexValuesIfNeeded(tblReaderExec.ctx.GetSessionVars().StmtCtx, 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) } diff --git a/table/tables/index.go b/table/tables/index.go index 8fb3a9f80256c..c9acd52732e1f 100644 --- a/table/tables/index.go +++ b/table/tables/index.go @@ -124,8 +124,10 @@ 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(sc *stmtctx.StatementContext, tblInfo *model.TableInfo, idxInfo *model.IndexInfo, indexedValues []types.Datum) ([]types.Datum, error) { +// 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] switch v.Kind() { @@ -185,7 +187,8 @@ 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, err = TruncateIndexValuesIfNeeded(sc, 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 } From dd4cb555c1331255b660cbd5c0d0ea00e8d8f390 Mon Sep 17 00:00:00 2001 From: crazycs Date: Tue, 4 Feb 2020 12:33:38 +0800 Subject: [PATCH 03/11] add check for only enlarge decimal size --- ddl/db_test.go | 4 ++++ ddl/ddl_api.go | 27 ++++++++++++++++----------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 72e3bf0300ef3..846fa694a7dfa 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3705,6 +3705,10 @@ func (s *testDBSuite1) TestModifyColumnWithDecimal(c *C) { 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');") diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 025daf0a493f7..4601e1bc6d02f 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2566,20 +2566,20 @@ func modifiable(tbInfo *model.TableInfo, originalCol *model.ColumnInfo, to *type } } case mysql.TypeNewDecimal: - for _, indexInfo := range tbInfo.Indices { - containColumn := false - for _, col := range indexInfo.Columns { - if col.Name.L == originalCol.Name.L { - containColumn = true - break + if to.Flen != origin.Flen || to.Decimal != origin.Decimal { + 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 to.Flen != origin.Flen || to.Decimal != origin.Decimal { + 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") } - break } } default: @@ -2596,6 +2596,11 @@ func modifiable(tbInfo *model.TableInfo, originalCol *model.ColumnInfo, to *type 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) From df86029d20b0dfb19ff8623d40f0f4c3b8091a8e Mon Sep 17 00:00:00 2001 From: crazycs Date: Tue, 4 Feb 2020 13:19:52 +0800 Subject: [PATCH 04/11] gs --- ddl/column_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ddl/column_test.go b/ddl/column_test.go index 769bb0bc72b87..746cc757c7fa2 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -938,7 +938,7 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { ) defer d.Stop() - tblInfo := testTableInfo(c, d, "t", 4) + tblInfo := testTableInfo(c, d, "t", 2) idx := &model.IndexInfo{ ID: 1, Name: model.NewCIStr("idx"), @@ -965,8 +965,9 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { {"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)", nil, tblInfo.Columns[1]}, + {"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]}, } for _, tt := range tests { ftA := s.colDefStrToFieldType(c, tt.origin) From e0d6d0e0135e2b88319ef14687ed8e7bd0be7b15 Mon Sep 17 00:00:00 2001 From: crazycs Date: Tue, 4 Feb 2020 13:20:04 +0800 Subject: [PATCH 05/11] refine test --- ddl/column_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddl/column_test.go b/ddl/column_test.go index 746cc757c7fa2..595d9ffd34291 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -944,7 +944,7 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { Name: model.NewCIStr("idx"), Columns: []*model.IndexColumn{ { - Name: tblInfo.Columns[0].Name, + Name: tblInfo.Columns[0].Name, // column[0] has index covered. }, }, } @@ -967,7 +967,7 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { {"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]}, + {"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) From 687ab785c3272bd46e0545c39716df01f172aed3 Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 5 Feb 2020 17:12:18 +0800 Subject: [PATCH 06/11] add decimal round for modify decimal when decode decimal data --- ddl/db_test.go | 33 ++++++++++++++------- executor/mem_reader.go | 6 ++++ server/http_handler_test.go | 2 +- store/mockstore/mocktikv/analyze.go | 3 ++ store/mockstore/mocktikv/cop_handler_dag.go | 3 ++ tablecodec/tablecodec.go | 4 +-- util/codec/codec.go | 30 ++++++++++++------- util/codec/decimal.go | 16 ++++++++++ util/rowcodec/decoder.go | 4 +-- 9 files changed, 76 insertions(+), 25 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 846fa694a7dfa..e332cbe65500c 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3714,27 +3714,40 @@ func (s *testDBSuite1) TestModifyColumnWithDecimal(c *C) { 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.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.MustQuery("select * from t_decimal ignore index(idx4) where b=12345.67891").Check(testkit.Rows("1 12345.678910 a")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=123.456").Check(testkit.Rows("2 123.456000 b")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=12345.678912").Check(testkit.Rows("3 12345.678912 c")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=0").Check(testkit.Rows("4 0.000000 d")) + s.tk.MustQuery("select b from t_decimal ignore index(idx4) where b=12345.67891").Check(testkit.Rows("12345.678910")) + s.tk.MustQuery("select b from t_decimal ignore index(idx4) where b=123.456").Check(testkit.Rows("123.456000")) + s.tk.MustQuery("select b from t_decimal ignore index(idx4) where b=12345.678912").Check(testkit.Rows("12345.678912")) + s.tk.MustQuery("select b from t_decimal ignore 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 * 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.MustQuery("select * from t_decimal ignore 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")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=12346.67891").Check(testkit.Rows("6 12346.678910 a")) + s.tk.MustQuery("select b from t_decimal ignore 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 ")) + 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 ")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=123.456 order by b").Check(testkit.Rows("2 123.456000 b", "7 123.456000 ")) 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") diff --git a/executor/mem_reader.go b/executor/mem_reader.go index 01f86570d020f..331b4d2153085 100644 --- a/executor/mem_reader.go +++ b/executor/mem_reader.go @@ -163,6 +163,9 @@ func buildMemTableReader(us *UnionScanExec, tblReader *TableReaderExecutor) *mem Tp: int32(col.Tp), Flag: int32(col.Flag), IsPKHandle: us.table.Meta().PKIsHandle && mysql.HasPriKeyFlag(col.Flag), + Flen: col.Flen, + Decimal: col.Decimal, + Elems: col.Elems, }) } @@ -413,6 +416,9 @@ func (m *memIndexLookUpReader) getMemRows() ([][]types.Datum, error) { Tp: int32(col.Tp), Flag: int32(col.Flag), IsPKHandle: m.table.Meta().PKIsHandle && mysql.HasPriKeyFlag(col.Flag), + Flen: col.Flen, + Decimal: col.Decimal, + Elems: col.Elems, }) } rd := rowcodec.NewByteDecoder(colInfos, -1, nil, nil) diff --git a/server/http_handler_test.go b/server/http_handler_test.go index 5160e4468d31f..a92bd4736e57b 100644 --- a/server/http_handler_test.go +++ b/server/http_handler_test.go @@ -590,7 +590,7 @@ func (ts *HTTPHandlerTestSuite) TestDecodeColumnValue(c *C) { row := make([]types.Datum, len(cols)) row[0] = types.NewIntDatum(100) row[1] = types.NewBytesDatum([]byte("abc")) - row[2] = types.NewDecimalDatum(types.NewDecFromInt(1)) + row[2] = types.NewDecimalDatum(types.NewDecFromStringForTest("1.123456")) row[3] = types.NewTimeDatum(types.NewTime(types.FromGoTime(time.Now()), mysql.TypeTimestamp, 6)) // Encode the row. diff --git a/store/mockstore/mocktikv/analyze.go b/store/mockstore/mocktikv/analyze.go index 30dfe59ea5141..6a2d2f179ba32 100644 --- a/store/mockstore/mocktikv/analyze.go +++ b/store/mockstore/mocktikv/analyze.go @@ -148,6 +148,9 @@ func (h *rpcHandler) handleAnalyzeColumnsReq(req *coprocessor.Request, analyzeRe Tp: col.Tp, Flag: col.Flag, IsPKHandle: col.GetPkHandle(), + Flen: int(col.ColumnLen), + Decimal: int(col.Decimal), + Elems: col.Elems, } } defVal := func(i int) ([]byte, error) { diff --git a/store/mockstore/mocktikv/cop_handler_dag.go b/store/mockstore/mocktikv/cop_handler_dag.go index c7b1a1722697c..743f0990a3e8f 100644 --- a/store/mockstore/mocktikv/cop_handler_dag.go +++ b/store/mockstore/mocktikv/cop_handler_dag.go @@ -208,6 +208,9 @@ func (h *rpcHandler) buildTableScan(ctx *dagContext, executor *tipb.Executor) (* Tp: col.Tp, Flag: col.Flag, IsPKHandle: col.GetPkHandle(), + Flen: int(col.ColumnLen), + Decimal: int(col.Decimal), + Elems: col.Elems, } } defVal := func(i int) ([]byte, error) { diff --git a/tablecodec/tablecodec.go b/tablecodec/tablecodec.go index 671a5a624dbdd..e1121f76e524a 100644 --- a/tablecodec/tablecodec.go +++ b/tablecodec/tablecodec.go @@ -335,7 +335,7 @@ func flatten(sc *stmtctx.StatementContext, data types.Datum, ret *types.Datum) e // DecodeColumnValue decodes data to a Datum according to the column info. func DecodeColumnValue(data []byte, ft *types.FieldType, loc *time.Location) (types.Datum, error) { - _, d, err := codec.DecodeOne(data) + _, d, err := codec.DecodeOneWithTp(data, ft) if err != nil { return types.Datum{}, errors.Trace(err) } @@ -413,7 +413,7 @@ func DecodeRowWithMap(b []byte, cols map[int64]*types.FieldType, loc *time.Locat id := cid.GetInt64() ft, ok := cols[id] if ok { - _, v, err := codec.DecodeOne(data) + _, v, err := codec.DecodeOneWithTp(data, ft) if err != nil { return nil, errors.Trace(err) } diff --git a/util/codec/codec.go b/util/codec/codec.go index d7bf3b934a9c9..b68dcf6642095 100644 --- a/util/codec/codec.go +++ b/util/codec/codec.go @@ -714,6 +714,25 @@ func DecodeRange(b []byte, size int) ([]types.Datum, []byte, error) { return values, nil, nil } +// DecodeOneWithTp decodes on datum from a byte slice generated with EncodeKey or EncodeValue. +func DecodeOneWithTp(b []byte, ft *types.FieldType) (remain []byte, d types.Datum, err error) { + remain, d, err = DecodeOne(b) + if ft.Tp != mysql.TypeNewDecimal || err != nil { + return remain, d, err + } + if ft.Decimal != types.UnspecifiedLength && d.Frac() != ft.Decimal { + dec := d.GetMysqlDecimal() + to := new(types.MyDecimal) + err = dec.Round(to, ft.Decimal, types.ModeHalfEven) + if err != nil { + return remain, d, err + } + d.SetMysqlDecimal(to) + d.SetFrac(ft.Decimal) + } + return remain, d, err +} + // DecodeOne decodes on datum from a byte slice generated with EncodeKey or EncodeValue. func DecodeOne(b []byte) (remain []byte, d types.Datum, err error) { if len(b) < 1 { @@ -984,19 +1003,10 @@ func (decoder *Decoder) DecodeOne(b []byte, colIdx int, ft *types.FieldType) (re chk.AppendBytes(colIdx, v) case decimalFlag: var dec *types.MyDecimal - var frac int - b, dec, _, frac, err = DecodeDecimal(b) + b, dec, _, _, err = DecodeDecimalAndRound(b, ft.Decimal) if err != nil { return nil, errors.Trace(err) } - if ft.Decimal != types.UnspecifiedLength && frac != ft.Decimal { - to := new(types.MyDecimal) - err := dec.Round(to, ft.Decimal, types.ModeHalfEven) - if err != nil { - return nil, errors.Trace(err) - } - dec = to - } chk.AppendMyDecimal(colIdx, dec) case durationFlag: var r int64 diff --git a/util/codec/decimal.go b/util/codec/decimal.go index 2c1668d2b8b5c..8b0ecae510022 100644 --- a/util/codec/decimal.go +++ b/util/codec/decimal.go @@ -59,3 +59,19 @@ func DecodeDecimal(b []byte) ([]byte, *types.MyDecimal, int, int, error) { } return b, dec, precision, frac, nil } + +func DecodeDecimalAndRound(b []byte, decimal int) ([]byte, *types.MyDecimal, int, int, error) { + b, dec, precision, frac, err := DecodeDecimal(b) + if err != nil { + return b, dec, precision, frac, err + } + if decimal != types.UnspecifiedLength && frac != decimal { + to := new(types.MyDecimal) + err = dec.Round(to, decimal, types.ModeHalfEven) + if err != nil { + return b, dec, precision, frac, err + } + dec = to + } + return b, dec, precision, frac, err +} diff --git a/util/rowcodec/decoder.go b/util/rowcodec/decoder.go index 08f8baab48277..aad9178023a0e 100644 --- a/util/rowcodec/decoder.go +++ b/util/rowcodec/decoder.go @@ -133,7 +133,7 @@ func (decoder *DatumMapDecoder) decodeColDatum(col *ColInfo, colData []byte) (ty mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: d.SetBytes(colData) case mysql.TypeNewDecimal: - _, dec, _, _, err := codec.DecodeDecimal(colData) + _, dec, _, _, err := codec.DecodeDecimalAndRound(colData, col.Decimal) if err != nil { return d, err } @@ -268,7 +268,7 @@ func (decoder *ChunkDecoder) decodeColToChunk(colIdx int, col *ColInfo, colData mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: chk.AppendBytes(colIdx, colData) case mysql.TypeNewDecimal: - _, dec, _, _, err := codec.DecodeDecimal(colData) + _, dec, _, _, err := codec.DecodeDecimalAndRound(colData, col.Decimal) if err != nil { return err } From 18a7d5745fbe1751f30c82f2b56367c8075bf19b Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 5 Feb 2020 19:10:29 +0800 Subject: [PATCH 07/11] make ci happy --- util/codec/decimal.go | 1 + 1 file changed, 1 insertion(+) diff --git a/util/codec/decimal.go b/util/codec/decimal.go index 8b0ecae510022..656b7de84f656 100644 --- a/util/codec/decimal.go +++ b/util/codec/decimal.go @@ -60,6 +60,7 @@ func DecodeDecimal(b []byte) ([]byte, *types.MyDecimal, int, int, error) { return b, dec, precision, frac, nil } +// DecodeDecimalAndRound decodes bytes to decimal and round. func DecodeDecimalAndRound(b []byte, decimal int) ([]byte, *types.MyDecimal, int, int, error) { b, dec, precision, frac, err := DecodeDecimal(b) if err != nil { From 5453489ff499c0088cd93329e6b22f6775647533 Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 5 Feb 2020 19:28:54 +0800 Subject: [PATCH 08/11] fix ci test --- util/codec/codec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/codec/codec.go b/util/codec/codec.go index b68dcf6642095..6855a06658710 100644 --- a/util/codec/codec.go +++ b/util/codec/codec.go @@ -720,7 +720,7 @@ func DecodeOneWithTp(b []byte, ft *types.FieldType) (remain []byte, d types.Datu if ft.Tp != mysql.TypeNewDecimal || err != nil { return remain, d, err } - if ft.Decimal != types.UnspecifiedLength && d.Frac() != ft.Decimal { + if ft.Decimal != types.UnspecifiedLength && d.Frac() != ft.Decimal && d.GetInterface() != nil { dec := d.GetMysqlDecimal() to := new(types.MyDecimal) err = dec.Round(to, ft.Decimal, types.ModeHalfEven) From 0bbff10d6abec9cad7040015bd0457dce109a5ac Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 5 Feb 2020 19:48:50 +0800 Subject: [PATCH 09/11] add test for point get --- ddl/db_test.go | 64 +++++++++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index ec3e6e4f83c73..e456cbcb5e887 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3694,8 +3694,8 @@ func (s *testDBSuite1) TestModifyColumnCharset(c *C) { 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');") + s.tk.MustExec("create table t_decimal (a int,b decimal(10,5),c int key, index idx1(b),index idx2(a,b,c),index idx3(a));") + s.tk.MustExec("insert into t_decimal values (1,12345.67891,1),(2,123.456,2),(4,0.0,4);") // 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);", @@ -3717,50 +3717,60 @@ func (s *testDBSuite1) TestModifyColumnWithDecimal(c *C) { // 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("insert into t_decimal values (3, 12345.678912, 3);") + s.tk.MustQuery("select * from t_decimal order by a").Check(testkit.Rows("1 12345.678910 1", "2 123.456000 2", "3 12345.678912 3", "4 0.000000 4")) 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")) + // Test for index lookup reader. + s.tk.MustQuery("select * from t_decimal use index(idx4) where b=12345.67891").Check(testkit.Rows("1 12345.678910 1")) + s.tk.MustQuery("select * from t_decimal use index(idx4) where b=123.456").Check(testkit.Rows("2 123.456000 2")) + s.tk.MustQuery("select * from t_decimal use index(idx4) where b=12345.678912").Check(testkit.Rows("3 12345.678912 3")) + s.tk.MustQuery("select * from t_decimal use index(idx4) where b=0").Check(testkit.Rows("4 0.000000 4")) + // Test for index reader. 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.MustQuery("select * from t_decimal ignore index(idx4) where b=12345.67891").Check(testkit.Rows("1 12345.678910 a")) - s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=123.456").Check(testkit.Rows("2 123.456000 b")) - s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=12345.678912").Check(testkit.Rows("3 12345.678912 c")) - s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=0").Check(testkit.Rows("4 0.000000 d")) - s.tk.MustQuery("select b from t_decimal ignore index(idx4) where b=12345.67891").Check(testkit.Rows("12345.678910")) - s.tk.MustQuery("select b from t_decimal ignore index(idx4) where b=123.456").Check(testkit.Rows("123.456000")) - s.tk.MustQuery("select b from t_decimal ignore index(idx4) where b=12345.678912").Check(testkit.Rows("12345.678912")) - s.tk.MustQuery("select b from t_decimal ignore index(idx4) where b=0").Check(testkit.Rows("0.000000")) + // Test for table scan. + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=12345.67891").Check(testkit.Rows("1 12345.678910 1")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=123.456").Check(testkit.Rows("2 123.456000 2")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=12345.678912").Check(testkit.Rows("3 12345.678912 3")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=0").Check(testkit.Rows("4 0.000000 4")) 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 * from t_decimal use index(idx4) where b=12345.67891").Check(testkit.Rows("6 12345.678910 1")) s.tk.MustQuery("select b from t_decimal use index(idx4) where b=12345.67891").Check(testkit.Rows("12345.678910")) - s.tk.MustQuery("select * from t_decimal ignore 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.MustQuery("select * from t_decimal ignore index(idx4) where b=12345.67891").Check(testkit.Rows("6 12345.678910 1")) 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 * from t_decimal use index(idx4) where b=12346.67891").Check(testkit.Rows("6 12346.678910 1")) s.tk.MustQuery("select b from t_decimal use index(idx4) where b=12346.67891").Check(testkit.Rows("12346.678910")) - s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=12346.67891").Check(testkit.Rows("6 12346.678910 a")) - s.tk.MustQuery("select b from t_decimal ignore index(idx4) where b=12346.67891").Check(testkit.Rows("12346.678910")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=12346.67891").Check(testkit.Rows("6 12346.678910 1")) // 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 ")) - s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=123.456 order by b").Check(testkit.Rows("2 123.456000 b", "7 123.456000 ")) + s.tk.MustExec("insert into t_decimal values (7,123.456,7);") + s.tk.MustQuery("select * from t_decimal use index(idx4) where b=123.456 order by b").Check(testkit.Rows("2 123.456000 2", "7 123.456000 7")) + s.tk.MustQuery("select * from t_decimal ignore index(idx4) where b=123.456 order by b").Check(testkit.Rows("2 123.456000 2", "7 123.456000 7")) 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');", + s.tk.MustExec("insert into t_decimal values (8,123.456,8);") + testExecErrorMessage(c, s.tk, "insert into t_decimal values (9,123.456,1000);", "[kv:1062]Duplicate entry '123.456000' for key 'idx6'") + + // Test for point get double read. + s.tk.MustQuery("select * from t_decimal where b=12346.67891").Check(testkit.Rows("6 12346.678910 1")) + s.tk.MustQuery("select * from t_decimal where b=12345.678912").Check(testkit.Rows("3 12345.678912 3")) + s.tk.MustQuery("select * from t_decimal where b=0").Check(testkit.Rows("4 0.000000 4")) + s.tk.MustQuery("select * from t_decimal where b=123.456").Check(testkit.Rows("8 123.456000 8")) + + // Test for point get without double read. + s.tk.MustQuery("select * from t_decimal where c=1").Check(testkit.Rows("6 12346.678910 1")) + s.tk.MustQuery("select * from t_decimal where c=3").Check(testkit.Rows("3 12345.678912 3")) + s.tk.MustQuery("select * from t_decimal where c=4").Check(testkit.Rows("4 0.000000 4")) + s.tk.MustQuery("select * from t_decimal where c=8").Check(testkit.Rows("8 123.456000 8")) + s.tk.MustExec("admin check table t_decimal;") } From 6df3f9dafdbb9284d70adba5846daa89caa0a7c5 Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 6 Feb 2020 11:20:04 +0800 Subject: [PATCH 10/11] refine code Signed-off-by: crazycs --- util/codec/codec.go | 11 ++++++++++- util/codec/decimal.go | 6 +++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/util/codec/codec.go b/util/codec/codec.go index 6855a06658710..2476ca307c505 100644 --- a/util/codec/codec.go +++ b/util/codec/codec.go @@ -1003,10 +1003,19 @@ func (decoder *Decoder) DecodeOne(b []byte, colIdx int, ft *types.FieldType) (re chk.AppendBytes(colIdx, v) case decimalFlag: var dec *types.MyDecimal - b, dec, _, _, err = DecodeDecimalAndRound(b, ft.Decimal) + var frac int + b, dec, _, frac, err = DecodeDecimalAndRound(b, ft.Decimal) if err != nil { return nil, errors.Trace(err) } + if ft.Decimal != types.UnspecifiedLength && frac > ft.Decimal { + to := new(types.MyDecimal) + err := dec.Round(to, ft.Decimal, types.ModeHalfEven) + if err != nil { + return nil, errors.Trace(err) + } + dec = to + } chk.AppendMyDecimal(colIdx, dec) case durationFlag: var r int64 diff --git a/util/codec/decimal.go b/util/codec/decimal.go index 656b7de84f656..3efa3bb231a74 100644 --- a/util/codec/decimal.go +++ b/util/codec/decimal.go @@ -66,13 +66,13 @@ func DecodeDecimalAndRound(b []byte, decimal int) ([]byte, *types.MyDecimal, int if err != nil { return b, dec, precision, frac, err } - if decimal != types.UnspecifiedLength && frac != decimal { + if decimal != types.UnspecifiedLength && decimal > frac { to := new(types.MyDecimal) err = dec.Round(to, decimal, types.ModeHalfEven) if err != nil { return b, dec, precision, frac, err } - dec = to + return b, to, precision, decimal, nil } - return b, dec, precision, frac, err + return b, dec, precision, frac, nil } From c393b7e72a1d53adc4d5771fe58a740d9048095d Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 6 Feb 2020 13:56:33 +0800 Subject: [PATCH 11/11] fix insert on duplicate update value out of range bug and add test --- ddl/db_test.go | 18 +++++++++++++++++- util/codec/codec.go | 3 +++ util/rowcodec/decoder.go | 3 +++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index e456cbcb5e887..8c0b362801b91 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3694,6 +3694,7 @@ func (s *testDBSuite1) TestModifyColumnCharset(c *C) { func (s *testDBSuite1) TestModifyColumnWithDecimal(c *C) { s.tk = testkit.NewTestKit(c, s.store) s.tk.MustExec("use test_db;") + s.tk.MustExec("drop table if exists t_decimal") s.tk.MustExec("create table t_decimal (a int,b decimal(10,5),c int key, index idx1(b),index idx2(a,b,c),index idx3(a));") s.tk.MustExec("insert into t_decimal values (1,12345.67891,1),(2,123.456,2),(4,0.0,4);") // Test modify decimal column with index covered. @@ -3770,8 +3771,23 @@ func (s *testDBSuite1) TestModifyColumnWithDecimal(c *C) { s.tk.MustQuery("select * from t_decimal where c=3").Check(testkit.Rows("3 12345.678912 3")) s.tk.MustQuery("select * from t_decimal where c=4").Check(testkit.Rows("4 0.000000 4")) s.tk.MustQuery("select * from t_decimal where c=8").Check(testkit.Rows("8 123.456000 8")) - s.tk.MustExec("admin check table t_decimal;") + + // Test for update with point get. + s.tk.MustExec("drop table if exists t") + s.tk.MustExec("create table t (a decimal(24,5),b int key)") + s.tk.MustExec("insert into t values (7967475648705613894.87216,1)") + s.tk.MustExec("alter table t change a a decimal(64,17);") + s.tk.MustExec("update t set b=b+1 where b=1;") + s.tk.MustExec("admin check table t;") + + // Test for insert on duplicate key. + s.tk.MustExec("drop table if exists t") + s.tk.MustExec("create table t (a decimal(24,5),b int key)") + s.tk.MustExec("insert into t values (7967475648705613894.87216,1)") + s.tk.MustExec("alter table t change a a decimal(64,17);") + s.tk.MustExec("insert into t set b=1 on duplicate key update b=b+1;") + s.tk.MustExec("admin check table t;") } func testExecErrorMessage(c *C, tk *testkit.TestKit, sql, errMsg string) { diff --git a/util/codec/codec.go b/util/codec/codec.go index 2476ca307c505..d11e7ce0c2a17 100644 --- a/util/codec/codec.go +++ b/util/codec/codec.go @@ -730,6 +730,9 @@ func DecodeOneWithTp(b []byte, ft *types.FieldType) (remain []byte, d types.Datu d.SetMysqlDecimal(to) d.SetFrac(ft.Decimal) } + if ft.Flen > d.Length() { + d.SetLength(ft.Flen) + } return remain, d, err } diff --git a/util/rowcodec/decoder.go b/util/rowcodec/decoder.go index c6e9dab61b694..c3c82904728ae 100644 --- a/util/rowcodec/decoder.go +++ b/util/rowcodec/decoder.go @@ -138,6 +138,9 @@ func (decoder *DatumMapDecoder) decodeColDatum(col *ColInfo, colData []byte) (ty return d, err } d.SetMysqlDecimal(dec) + if col.Flen > precision { + precision = col.Flen + } d.SetLength(precision) d.SetFrac(frac) case mysql.TypeDate, mysql.TypeDatetime, mysql.TypeTimestamp: