From 01108075eb30433343470f163109046c44026bcc Mon Sep 17 00:00:00 2001 From: siddontang Date: Mon, 14 Sep 2015 16:55:28 +0800 Subject: [PATCH 1/5] types: support divide overflow check --- util/types/overflow.go | 36 +++++++++++++++++++ util/types/overflow_test.go | 70 +++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/util/types/overflow.go b/util/types/overflow.go index 94244eb1bdc9e..2e0efaddf47e0 100644 --- a/util/types/overflow.go +++ b/util/types/overflow.go @@ -151,3 +151,39 @@ func MulInteger(a uint64, b int64) (uint64, error) { return MulUint64(a, uint64(b)) } + +// DivInt64 divide int64 a with b, returns int64 if no overflow error. +// DivInt64 just checks overflow, if b is zero, a "divide by zero" panic throws. +func DivInt64(a int64, b int64) (int64, error) { + if a == math.MinInt64 && b == -1 { + return 0, errors.Trace(ErrArithOverflow) + } + + return a / b, nil +} + +// DivUintWithInt divide uint64 a with int64 b, returns uint64 if no overflow error. +func DivUintWithInt(a uint64, b int64) (uint64, error) { + if b < 0 { + if a != 0 && uint64(-b) <= a { + return 0, errors.Trace(ErrArithOverflow) + } + + return 0, nil + } + + return a / uint64(b), nil +} + +// DivIntWithUint divide int64 a with uint64 b, returns uint64 if no overflow error. +func DivIntWithUint(a int64, b uint64) (uint64, error) { + if a < 0 { + if uint64(-a) >= b { + return 0, errors.Trace(ErrArithOverflow) + } + + return 0, nil + } + + return uint64(a) / b, nil +} diff --git a/util/types/overflow_test.go b/util/types/overflow_test.go index a8d8b9d8fca94..4f5a622c80b3b 100644 --- a/util/types/overflow_test.go +++ b/util/types/overflow_test.go @@ -263,3 +263,73 @@ func (s *testOverflowSuite) TestMul(c *C) { } } } + +func (s *testOverflowSuite) TestDiv(c *C) { + tblInt64 := []struct { + lsh int64 + rsh int64 + ret int64 + overflow bool + }{ + {math.MaxInt64, 1, math.MaxInt64, false}, + {math.MinInt64, 1, math.MinInt64, false}, + {math.MinInt64, -1, 0, true}, + {math.MaxInt64, -1, -math.MaxInt64, false}, + {1, -1, -1, false}, + {-1, 1, -1, false}, + {-1, 2, 0, false}, + {math.MinInt64, 2, math.MinInt64 / 2, false}, + } + + for _, t := range tblInt64 { + ret, err := DivInt64(t.lsh, t.rsh) + if t.overflow { + c.Assert(err, NotNil) + } else { + c.Assert(ret, Equals, t.ret) + } + } + + tblInt := []struct { + lsh uint64 + rsh int64 + ret uint64 + overflow bool + }{ + {0, -1, 0, false}, + {1, -1, 0, true}, + {math.MaxInt64, math.MinInt64, 0, false}, + {math.MaxInt64, -1, 0, true}, + } + + for _, t := range tblInt { + ret, err := DivUintWithInt(t.lsh, t.rsh) + if t.overflow { + c.Assert(err, NotNil) + } else { + c.Assert(ret, Equals, t.ret) + } + } + + tblInt2 := []struct { + lsh int64 + rsh uint64 + ret uint64 + overflow bool + }{ + {math.MinInt64, math.MaxInt64, 0, true}, + {0, 1, 0, false}, + {-1, math.MaxInt64, 0, false}, + } + + for _, t := range tblInt2 { + c.Log(t.lsh, t.rsh) + + ret, err := DivIntWithUint(t.lsh, t.rsh) + if t.overflow { + c.Assert(err, NotNil) + } else { + c.Assert(ret, Equals, t.ret) + } + } +} From 7b38b74d5520a58209e15b49d987679e542bc834 Mon Sep 17 00:00:00 2001 From: siddontang Date: Mon, 14 Sep 2015 16:56:01 +0800 Subject: [PATCH 2/5] expressions: update integer divide and mod overflow check --- expression/expressions/binop.go | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/expression/expressions/binop.go b/expression/expressions/binop.go index 5a0a05ec84f9d..8b3f922b0af7a 100644 --- a/expression/expressions/binop.go +++ b/expression/expressions/binop.go @@ -554,14 +554,12 @@ func (o *BinaryOperation) evalIntDiv(a interface{}, b interface{}) (interface{}, if y == 0 { return nil, nil } - return x / y, nil + return types.DivInt64(x, y) case uint64: if y == 0 { return nil, nil } - // For MySQL, if any is unsigned, return unsigned - // TODO: check overflow - return uint64(x) / y, nil + return types.DivIntWithUint(x, y) } case uint64: switch y := b.(type) { @@ -569,9 +567,7 @@ func (o *BinaryOperation) evalIntDiv(a interface{}, b interface{}) (interface{}, if y == 0 { return nil, nil } - // For MySQL, if any is unsigned, return unsigned - // TODO: check overflow - return x / uint64(y), nil + return types.DivUintWithInt(x, y) case uint64: if y == 0 { return nil, nil @@ -611,11 +607,10 @@ func (o *BinaryOperation) evalMod(a interface{}, b interface{}) (interface{}, er if y == 0 { return nil, nil } else if x < 0 { - // TODO: check overflow + // first is int64, return int64. return -int64(uint64(-x) % y), nil } - // TODO: check overflow - return uint64(x) % y, nil + return int64(uint64(x) % y), nil } case uint64: switch y := b.(type) { @@ -623,10 +618,9 @@ func (o *BinaryOperation) evalMod(a interface{}, b interface{}) (interface{}, er if y == 0 { return nil, nil } else if y < 0 { - // TODO: check overflow - return -int64(x % uint64(-y)), nil + // first is uint64, return uint64. + return uint64(x % uint64(-y)), nil } - // TODO: check overflow return x % uint64(y), nil case uint64: if y == 0 { From 10d5f4b844521dd3a875384997d824b84a488975 Mon Sep 17 00:00:00 2001 From: siddontang Date: Mon, 14 Sep 2015 16:58:51 +0800 Subject: [PATCH 3/5] expressions: remove unnecessary comment --- expression/expressions/binop.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/expression/expressions/binop.go b/expression/expressions/binop.go index 8b3f922b0af7a..2cd2bf00d4585 100644 --- a/expression/expressions/binop.go +++ b/expression/expressions/binop.go @@ -411,7 +411,6 @@ func (o *BinaryOperation) evalComparisonOp(ctx context.Context, args map[interfa } func (o *BinaryOperation) evalPlus(a interface{}, b interface{}) (interface{}, error) { - // TODO: check overflow switch x := a.(type) { case int64: switch y := b.(type) { @@ -443,7 +442,6 @@ func (o *BinaryOperation) evalPlus(a interface{}, b interface{}) (interface{}, e } func (o *BinaryOperation) evalMinus(a interface{}, b interface{}) (interface{}, error) { - // TODO: check overflow switch x := a.(type) { case int64: switch y := b.(type) { @@ -475,7 +473,6 @@ func (o *BinaryOperation) evalMinus(a interface{}, b interface{}) (interface{}, } func (o *BinaryOperation) evalMul(a interface{}, b interface{}) (interface{}, error) { - // TODO: check overflow switch x := a.(type) { case int64: switch y := b.(type) { From 5844708988567b40bfc3f279350207748ced23a4 Mon Sep 17 00:00:00 2001 From: siddontang Date: Mon, 14 Sep 2015 17:03:01 +0800 Subject: [PATCH 4/5] types: Address comment. --- util/types/overflow.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/util/types/overflow.go b/util/types/overflow.go index 2e0efaddf47e0..f1c682c083bbe 100644 --- a/util/types/overflow.go +++ b/util/types/overflow.go @@ -152,8 +152,8 @@ func MulInteger(a uint64, b int64) (uint64, error) { return MulUint64(a, uint64(b)) } -// DivInt64 divide int64 a with b, returns int64 if no overflow error. -// DivInt64 just checks overflow, if b is zero, a "divide by zero" panic throws. +// DivInt64 divides int64 a with b, returns int64 if no overflow error. +// It just checks overflow, if b is zero, a "divide by zero" panic throws. func DivInt64(a int64, b int64) (int64, error) { if a == math.MinInt64 && b == -1 { return 0, errors.Trace(ErrArithOverflow) @@ -162,7 +162,8 @@ func DivInt64(a int64, b int64) (int64, error) { return a / b, nil } -// DivUintWithInt divide uint64 a with int64 b, returns uint64 if no overflow error. +// DivUintWithInt divides uint64 a with int64 b, returns uint64 if no overflow error. +// It just checks overflow, if b is zero, a "divide by zero" panic throws. func DivUintWithInt(a uint64, b int64) (uint64, error) { if b < 0 { if a != 0 && uint64(-b) <= a { @@ -175,7 +176,8 @@ func DivUintWithInt(a uint64, b int64) (uint64, error) { return a / uint64(b), nil } -// DivIntWithUint divide int64 a with uint64 b, returns uint64 if no overflow error. +// DivIntWithUint divides int64 a with uint64 b, returns uint64 if no overflow error. +// It just checks overflow, if b is zero, a "divide by zero" panic throws. func DivIntWithUint(a int64, b uint64) (uint64, error) { if a < 0 { if uint64(-a) >= b { From 88902c07463c1b2ba1a3c150c9c319227a5c2db5 Mon Sep 17 00:00:00 2001 From: siddontang Date: Mon, 14 Sep 2015 17:10:36 +0800 Subject: [PATCH 5/5] types: Address comment --- util/types/overflow_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/util/types/overflow_test.go b/util/types/overflow_test.go index 4f5a622c80b3b..27f6c44016d01 100644 --- a/util/types/overflow_test.go +++ b/util/types/overflow_test.go @@ -323,8 +323,6 @@ func (s *testOverflowSuite) TestDiv(c *C) { } for _, t := range tblInt2 { - c.Log(t.lsh, t.rsh) - ret, err := DivIntWithUint(t.lsh, t.rsh) if t.overflow { c.Assert(err, NotNil)