Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix compatibility bug with convert string to int return wrong result #7483

Merged
merged 8 commits into from
Aug 31, 2018
40 changes: 40 additions & 0 deletions expression/builtin_cast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,26 @@ func (s *testEvaluatorSuite) TestCast(c *C) {
lastWarn = warnings[len(warnings)-1]
c.Assert(terror.ErrorEqual(types.ErrTruncatedWrongVal, lastWarn.Err), IsTrue, Commentf("err %v", lastWarn.Err))

// cast('125e342.83' as unsigned)
f = BuildCastFunction(ctx, &Constant{Value: types.NewDatum("125e342.83"), RetType: types.NewFieldType(mysql.TypeString)}, tp1)
res, err = f.Eval(chunk.Row{})
c.Assert(err, IsNil)
c.Assert(res.GetUint64() == 125, IsTrue)

warnings = sc.GetWarnings()
lastWarn = warnings[len(warnings)-1]
c.Assert(terror.ErrorEqual(types.ErrOverflow, lastWarn.Err), IsTrue, Commentf("err %v", lastWarn.Err))

// cast('1e9223372036854775807' as unsigned)
f = BuildCastFunction(ctx, &Constant{Value: types.NewDatum("1e9223372036854775807"), RetType: types.NewFieldType(mysql.TypeString)}, tp1)
res, err = f.Eval(chunk.Row{})
c.Assert(err, IsNil)
c.Assert(res.GetUint64() == 1, IsTrue)

warnings = sc.GetWarnings()
lastWarn = warnings[len(warnings)-1]
c.Assert(terror.ErrorEqual(types.ErrOverflow, lastWarn.Err), IsTrue, Commentf("err %v", lastWarn.Err))

// cast('18446744073709551616' as signed);
mask := ^mysql.UnsignedFlag
tp1.Flag &= uint(mask)
Expand All @@ -149,6 +169,26 @@ func (s *testEvaluatorSuite) TestCast(c *C) {
lastWarn = warnings[len(warnings)-1]
c.Assert(terror.ErrorEqual(types.ErrCastAsSignedOverflow, lastWarn.Err), IsTrue, Commentf("err %v", lastWarn.Err))

// cast('125e342.83' as signed)
f = BuildCastFunction(ctx, &Constant{Value: types.NewDatum("125e342.83"), RetType: types.NewFieldType(mysql.TypeString)}, tp1)
res, err = f.Eval(chunk.Row{})
c.Assert(err, IsNil)
c.Assert(res.GetInt64() == 125, IsTrue)

warnings = sc.GetWarnings()
lastWarn = warnings[len(warnings)-1]
c.Assert(terror.ErrorEqual(types.ErrOverflow, lastWarn.Err), IsTrue, Commentf("err %v", lastWarn.Err))

// cast('1e9223372036854775807' as signed)
f = BuildCastFunction(ctx, &Constant{Value: types.NewDatum("1e9223372036854775807"), RetType: types.NewFieldType(mysql.TypeString)}, tp1)
res, err = f.Eval(chunk.Row{})
c.Assert(err, IsNil)
c.Assert(res.GetInt64() == 1, IsTrue)

warnings = sc.GetWarnings()
lastWarn = warnings[len(warnings)-1]
c.Assert(terror.ErrorEqual(types.ErrOverflow, lastWarn.Err), IsTrue, Commentf("err %v", lastWarn.Err))

// create table t1(s1 time);
// insert into t1 values('11:11:11');
// select cast(s1 as decimal(7, 2)) from t1;
Expand Down
10 changes: 6 additions & 4 deletions types/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,13 @@ func getValidIntPrefix(sc *stmtctx.StatementContext, str string) (string, error)
if err != nil {
return floatPrefix, errors.Trace(err)
}
return floatStrToIntStr(floatPrefix)
return floatStrToIntStr(sc, floatPrefix, str)
}

// floatStrToIntStr converts a valid float string into valid integer string which can be parsed by
// strconv.ParseInt, we can't parse float first then convert it to string because precision will
// be lost.
func floatStrToIntStr(validFloat string) (string, error) {
func floatStrToIntStr(sc *stmtctx.StatementContext, validFloat string, oriStr string) (string, error) {
var dotIdx = -1
var eIdx = -1
for i := 0; i < len(validFloat); i++ {
Expand Down Expand Up @@ -275,7 +275,8 @@ func floatStrToIntStr(validFloat string) (string, error) {
}
if exp > 0 && int64(intCnt) > (math.MaxInt64-int64(exp)) {
// (exp + incCnt) overflows MaxInt64.
return validFloat, ErrOverflow.GenByArgs("BIGINT", validFloat)
sc.AppendWarning(ErrOverflow.GenByArgs("BIGINT", oriStr))
return validFloat[:eIdx], nil
}
intCnt += exp
if intCnt <= 0 {
Expand All @@ -292,7 +293,8 @@ func floatStrToIntStr(validFloat string) (string, error) {
extraZeroCount := intCnt - len(digits)
if extraZeroCount > 20 {
// Return overflow to avoid allocating too much memory.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update this comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeRushing Please update this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zz-jason already updated

return validFloat, ErrOverflow.GenByArgs("BIGINT", validFloat)
sc.AppendWarning(ErrOverflow.GenByArgs("BIGINT", oriStr))
return validFloat[:eIdx], nil
}
validInt = string(digits) + strings.Repeat("0", extraZeroCount)
}
Expand Down
13 changes: 9 additions & 4 deletions types/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,10 +684,15 @@ func (s *testTypeConvertSuite) TestGetValidFloat(c *C) {
_, err := strconv.ParseFloat(prefix, 64)
c.Assert(err, IsNil)
}
_, err := floatStrToIntStr("1e9223372036854775807")
c.Assert(terror.ErrorEqual(err, ErrOverflow), IsTrue, Commentf("err %v", err))
_, err = floatStrToIntStr("1e21")
c.Assert(terror.ErrorEqual(err, ErrOverflow), IsTrue, Commentf("err %v", err))
floatStr, err := floatStrToIntStr(sc, "1e9223372036854775807", "1e9223372036854775807")
c.Assert(err, IsNil)
c.Assert(floatStr, Equals, "1")
floatStr, err = floatStrToIntStr(sc, "125e342", "125e342.83")
c.Assert(err, IsNil)
c.Assert(floatStr, Equals, "125")
floatStr, err = floatStrToIntStr(sc, "1e21", "1e21")
c.Assert(err, IsNil)
c.Assert(floatStr, Equals, "1")
}

// TestConvertTime tests time related conversion.
Expand Down