util/types: fix decimal conversion. (pingcap#1979)

When datum converts to decimal, we should check the Flen and Decimal of the given field type, return max value for overflow and round the value for truncation.
juniway · Nov 9, 2016 · 5d3bb33 · 5d3bb33
1 parent fa9cb73
commit 5d3bb33
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 11 deletions.
diff --git a/executor/executor_test.go b/executor/executor_test.go
@@ -903,7 +903,7 @@ func (s *testSuite) TestBuiltin(c *C) {
 	result.Check(testkit.Rows("3 2"))
 
 	// test cast
-	result = tk.MustQuery("select cast(1 as decimal(1,2))")
+	result = tk.MustQuery("select cast(1 as decimal(3,2))")
 	result.Check(testkit.Rows("1.00"))
 	result = tk.MustQuery("select cast('1991-09-05 11:11:11' as datetime)")
 	result.Check(testkit.Rows("1991-09-05 11:11:11"))
@@ -1027,6 +1027,7 @@ func (s *testSuite) TestToPBExpr(c *C) {
 	tk.MustExec("use test")
 	tk.MustExec("drop table if exists t")
 	tk.MustExec("create table t (a decimal(10,6), b decimal, index idx_b (b))")
+	tk.MustExec("set sql_mode = ''")
 	tk.MustExec("insert t values (1.1, 1.1)")
 	tk.MustExec("insert t values (2.4, 2.4)")
 	tk.MustExec("insert t values (3.3, 2.7)")
@@ -1078,6 +1079,7 @@ func (s *testSuite) TestDatumXAPI(c *C) {
 	tk.MustExec("use test")
 	tk.MustExec("drop table if exists t")
 	tk.MustExec("create table t (a decimal(10,6), b decimal, index idx_b (b))")
+	tk.MustExec("set sql_mode = ''")
 	tk.MustExec("insert t values (1.1, 1.1)")
 	tk.MustExec("insert t values (2.2, 2.2)")
 	tk.MustExec("insert t values (3.3, 2.7)")

diff --git a/mysql/mydecimal.go b/mysql/mydecimal.go
@@ -2093,3 +2093,20 @@ func NewDecFromStringForTest(s string) *MyDecimal {
 	dec.FromString([]byte(s))
 	return dec
 }
+
+// NewMaxOrMinDec returns the max or min value decimal for given precision and fraction.
+func NewMaxOrMinDec(negative bool, prec, frac int) *MyDecimal {
+	str := make([]byte, prec+2)
+	for i := 0; i < len(str); i++ {
+		str[i] = '9'
+	}
+	if negative {
+		str[0] = '-'
+	} else {
+		str[0] = '+'
+	}
+	str[1+prec-frac] = '.'
+	dec := new(MyDecimal)
+	dec.FromString(str)
+	return dec
+}
diff --git a/mysql/mydecimal_test.go b/mysql/mydecimal_test.go
@@ -641,3 +641,23 @@ func (s *testMyDecimalSuite) TestDivMod(c *C) {
 		c.Assert(to.String(), Equals, ca.result)
 	}
 }
+
+func (s *testMyDecimalSuite) TestMaxOrMin(c *C) {
+	type tcase struct {
+		neg    bool
+		prec   int
+		frac   int
+		result string
+	}
+	cases := []tcase{
+		{true, 2, 1, "-9.9"},
+		{false, 1, 1, "0.9"},
+		{true, 1, 0, "-9"},
+		{false, 0, 0, "0"},
+		{false, 4, 2, "99.99"},
+	}
+	for _, ca := range cases {
+		dec := NewMaxOrMinDec(ca.neg, ca.prec, ca.frac)
+		c.Assert(dec.String(), Equals, ca.result)
+	}
+}
diff --git a/plan/physical_plan_builder.go b/plan/physical_plan_builder.go
@@ -23,6 +23,7 @@ import (
 	"github.com/pingcap/tidb/model"
 	"github.com/pingcap/tidb/mysql"
 	"github.com/pingcap/tidb/plan/statistics"
+	"github.com/pingcap/tidb/terror"
 	"github.com/pingcap/tidb/util/types"
 )
 
@@ -231,7 +232,10 @@ func (p *DataSource) convert2IndexScan(prop *requiredProperty, index *model.Inde
 		}
 		err := buildIndexRange(is)
 		if err != nil {
-			return nil, errors.Trace(err)
+			if !terror.ErrorEqual(err, mysql.ErrTruncated) {
+				return nil, errors.Trace(err)
+			}
+			log.Warn("truncate error in buildIndexRange")
 		}
 		for _, idxRange := range is.Ranges {
 			cnt, err := getRowCountByIndexRange(statsTbl, idxRange, is.Index)

diff --git a/session.go b/session.go
@@ -828,7 +828,9 @@ func (s *session) loadCommonGlobalVariablesIfNeeded() error {
 			break
 		}
 		varName := row.Data[0].GetString()
-		vars.SetSystemVar(varName, row.Data[1])
+		if d := vars.GetSystemVar(varName); d.IsNull() {
+			vars.SetSystemVar(varName, row.Data[1])
+		}
 	}
 	vars.CommonGlobalLoaded = true
 	return nil

diff --git a/table/tables/tables_test.go b/table/tables/tables_test.go
@@ -137,7 +137,7 @@ func countEntriesWithPrefix(ctx context.Context, prefix []byte) (int, error) {
 }
 
 func (ts *testSuite) TestTypes(c *C) {
-	_, err := ts.se.Execute("CREATE TABLE test.t (c1 tinyint, c2 smallint, c3 int, c4 bigint, c5 text, c6 blob, c7 varchar(64), c8 time, c9 timestamp not null default CURRENT_TIMESTAMP, c10 decimal)")
+	_, err := ts.se.Execute("CREATE TABLE test.t (c1 tinyint, c2 smallint, c3 int, c4 bigint, c5 text, c6 blob, c7 varchar(64), c8 time, c9 timestamp not null default CURRENT_TIMESTAMP, c10 decimal(10,1))")
 	c.Assert(err, IsNil)
 	ctx := ts.se.(context.Context)
 	dom := sessionctx.GetDomain(ctx)

diff --git a/util/types/convert_test.go b/util/types/convert_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/juju/errors"
 	. "github.com/pingcap/check"
 	"github.com/pingcap/tidb/mysql"
+	"github.com/pingcap/tidb/terror"
 	"github.com/pingcap/tidb/util/charset"
 	"github.com/pingcap/tidb/util/testleak"
 )
@@ -198,10 +199,20 @@ func (s *testTypeConvertSuite) TestConvertType(c *C) {
 
 	// For TypeNewDecimal
 	ft = NewFieldType(mysql.TypeNewDecimal)
-	ft.Decimal = 5
-	v, err = Convert(3.1415926, ft)
-	c.Assert(err, IsNil)
-	c.Assert(v.(*mysql.MyDecimal).String(), Equals, "3.14159")
+	ft.Flen = 8
+	ft.Decimal = 4
+	v, err = Convert(3.1416, ft)
+	c.Assert(err, IsNil, Commentf(errors.ErrorStack(err)))
+	c.Assert(v.(*mysql.MyDecimal).String(), Equals, "3.1416")
+	v, err = Convert("3.1415926", ft)
+	c.Assert(terror.ErrorEqual(err, mysql.ErrTruncated), IsTrue)
+	c.Assert(v.(*mysql.MyDecimal).String(), Equals, "3.1416")
+	v, err = Convert("99999", ft)
+	c.Assert(terror.ErrorEqual(err, mysql.ErrOverflow), IsTrue)
+	c.Assert(v.(*mysql.MyDecimal).String(), Equals, "9999.9999")
+	v, err = Convert("-10000", ft)
+	c.Assert(terror.ErrorEqual(err, mysql.ErrOverflow), IsTrue)
+	c.Assert(v.(*mysql.MyDecimal).String(), Equals, "-9999.9999")
 
 	// For TypeYear
 	ft = NewFieldType(mysql.TypeYear)
@@ -283,9 +294,10 @@ func (s *testTypeConvertSuite) TestConvertToString(c *C) {
 	testToString(c, td, "11:11:11.999999")
 
 	ft := NewFieldType(mysql.TypeNewDecimal)
+	ft.Flen = 10
 	ft.Decimal = 5
 	v, err := Convert(3.1415926, ft)
-	c.Assert(err, IsNil)
+	c.Assert(terror.ErrorEqual(err, mysql.ErrTruncated), IsTrue)
 	testToString(c, v, "3.14159")
 
 	_, err = ToString(&invalidMockType{})

diff --git a/util/types/datum.go b/util/types/datum.go
@@ -974,8 +974,17 @@ func (d *Datum) convertToMysqlDecimal(target *FieldType) (Datum, error) {
 	default:
 		return invalidConv(d, target.Tp)
 	}
-	if target.Decimal != UnspecifiedLength {
-		dec.Round(dec, target.Decimal)
+	if target.Flen != UnspecifiedLength && target.Decimal != UnspecifiedLength {
+		prec, frac := dec.PrecisionAndFrac()
+		if prec-frac > target.Flen-target.Decimal {
+			dec = mysql.NewMaxOrMinDec(dec.IsNegative(), target.Flen, target.Decimal)
+			err = errors.Trace(mysql.ErrOverflow)
+		} else if frac != target.Decimal {
+			dec.Round(dec, target.Decimal)
+			if frac > target.Decimal {
+				err = errors.Trace(mysql.ErrTruncated)
+			}
+		}
 	}
 	ret.SetValue(dec)
 	return ret, err