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.

Sign up for GitHub

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

Jump to bottom

types: fix uint64 overflow bug in ConvertJSONToFloat #11355

Merged
merged 5 commits into from
Jul 25, 2019
Merged

types: fix uint64 overflow bug in ConvertJSONToFloat #11355

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8400b83
fix bug of convert json to float
H-ZeX Jul 22, 2019
424c307
fix bug of convert json to float
H-ZeX Jul 22, 2019
732e3ab
expression: add a integration
H-ZeX Jul 25, 2019
57fbc52
Merge branch 'master' into fix-uint-convert-bug
winkyao Jul 25, 2019
592c82d
Merge branch 'master' into fix-uint-convert-bug
sre-bot Jul 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion expression/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2172,10 +2172,14 @@ func (s *testIntegrationSuite) TestBuiltin(c *C) {
result.Check(testkit.Rows("<nil>"))
result = tk.MustQuery(`select cast(cast('2017-01-01 01:01:11.12' as date) as datetime(2));`)
result.Check(testkit.Rows("2017-01-01 00:00:00.00"))

result = tk.MustQuery(`select cast(20170118.999 as datetime);`)
result.Check(testkit.Rows("2017-01-18 00:00:00"))

tk.MustExec(`create table tb5(a bigint(64) unsigned, b double);`)
Copy link
Member

Choose a reason for hiding this comment

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

why not directly create a json column?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If directly creat json, then I can not get a uint field(#11386 (comment)).

Copy link
Member

Choose a reason for hiding this comment

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

how about casting '9223372036854775808' to uint then cast to json?

tk.MustExec(`insert into tb5 (a, b) values (9223372036854776000, 9223372036854776000);`)
tk.MustExec(`insert into tb5 (a, b) select * from (select cast(a as json) as a1, b from tb5) as t where t.a1 = t.b;`)
tk.MustExec(`drop table tb5;`)

// Test corner cases of cast string as datetime
result = tk.MustQuery(`select cast("170102034" as datetime);`)
result.Check(testkit.Rows("2017-01-02 03:04:00"))
Expand Down
3 changes: 1 addition & 2 deletions types/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,7 @@ func ConvertJSONToFloat(sc *stmtctx.StatementContext, j json.BinaryJSON) (float6
case json.TypeCodeInt64:
return float64(j.GetInt64()), nil
case json.TypeCodeUint64:
u, err := ConvertIntToUint(sc, j.GetInt64(), IntergerUnsignedUpperBound(mysql.TypeLonglong), mysql.TypeLonglong)
Copy link
Contributor

@winkyao winkyao Jul 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Should we consider ShouldClipToZero?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need to consider this.
we extract a uint from the json, it must not < 0.

// GetInt64 gets the int64 value.
func (bj BinaryJSON) GetInt64() int64 {
	return int64(endian.Uint64(bj.Value))
}

// GetUint64 gets the uint64 value.
func (bj BinaryJSON) GetUint64() uint64 {
	return endian.Uint64(bj.Value)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

need or needn't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

needn't

return float64(u), errors.Trace(err)
return float64(j.GetUint64()), nil
case json.TypeCodeFloat64:
return j.GetFloat64(), nil
case json.TypeCodeString:
Expand Down
30 changes: 16 additions & 14 deletions types/convert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,24 +828,26 @@ func (s *testTypeConvertSuite) TestConvertJSONToInt(c *C) {

func (s *testTypeConvertSuite) TestConvertJSONToFloat(c *C) {
var tests = []struct {
In string
In interface{}
Out float64
ty json.TypeCode
}{
{`{}`, 0},
{`[]`, 0},
{`3`, 3},
{`-3`, -3},
{`4.5`, 4.5},
{`true`, 1},
{`false`, 0},
{`null`, 0},
{`"hello"`, 0},
{`"123.456hello"`, 123.456},
{`"1234"`, 1234},
{make(map[string]interface{}, 0), 0, json.TypeCodeObject},
{make([]interface{}, 0), 0, json.TypeCodeArray},
{int64(3), 3, json.TypeCodeInt64},
{int64(-3), -3, json.TypeCodeInt64},
{uint64(1 << 63), 1 << 63, json.TypeCodeUint64},
{float64(4.5), 4.5, json.TypeCodeFloat64},
{true, 1, json.TypeCodeLiteral},
{false, 0, json.TypeCodeLiteral},
{nil, 0, json.TypeCodeLiteral},
{"hello", 0, json.TypeCodeString},
{"123.456hello", 123.456, json.TypeCodeString},
{"1234", 1234, json.TypeCodeString},
}
for _, tt := range tests {
j, err := json.ParseBinaryFromString(tt.In)
c.Assert(err, IsNil)
j := json.CreateBinary(tt.In)
c.Assert(j.TypeCode, Equals, tt.ty)
casted, _ := ConvertJSONToFloat(new(stmtctx.StatementContext), j)
c.Assert(casted, Equals, tt.Out)
}
Expand Down
1 change: 1 addition & 0 deletions types/datum_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ func (ts *testDatumSuite) TestToJSON(c *C) {
{NewStringDatum("[1, 2, 3]"), `[1, 2, 3]`, true},
{NewStringDatum("{}"), `{}`, true},
{mustParseTimeIntoDatum("2011-11-10 11:11:11.111111", mysql.TypeTimestamp, 6), `"2011-11-10 11:11:11.111111"`, true},
{NewStringDatum(`{"a": "9223372036854775809"}`), `{"a": "9223372036854775809"}`, true},

// can not parse JSON from this string, so error occurs.
{NewStringDatum("hello, 世界"), "", false},
Expand Down