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

Subtraction operation implementation #5140

Merged
merged 14 commits into from
Sep 12, 2019
Merged

Subtraction operation implementation #5140

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9e4ea01
sqltypes: Subtract functionality
Aug 13, 2019
2c30eb4
Fixed code coverage in arithmetic.go
Aug 27, 2019
6ab4c17
Fixed spacing between // and comment
Aug 27, 2019
2141b63
Fixed comment spacing and cleaned up code
Aug 27, 2019
829f3d3
Fixed error print statements
Aug 27, 2019
0a1a9af
Fixed arithmetic.go and arithmetic_test.go through comments via github
Aug 30, 2019
bbb4f97
Merge remote-tracking branch 'origin/master' into rk-newminus
Aug 30, 2019
1141f5a
Deleted extra comments and tests in arithmetic.go and arithmetic_test.go
Aug 30, 2019
667e410
Changed Error Checking for subtraction in intMinusUintWithError()
Sep 5, 2019
00b3a08
Fixed error checking in arithmetic.go under uintPlusIntWithError()
Sep 5, 2019
355c915
Changed error checking to account for corner cases
Sep 10, 2019
b37147c
Merge remote-tracking branch 'origin/master' into rk-newminus
Sep 10, 2019
dd2f51f
Added test for uint - (- int)
Sep 10, 2019
008395d
Merge remote-tracking branch 'origin/master' into rk-newminus
Sep 10, 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
98 changes: 90 additions & 8 deletions go/sqltypes/arithmetic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ import (
"vitess.io/vitess/go/vt/vterrors"
)

// TODO(sougou): change these functions to be more permissive.
// Most string to number conversions should quietly convert to 0.

// numeric represents a numeric value extracted from
// a Value, used for arithmetic operations.
type numeric struct {
Expand All @@ -50,8 +47,14 @@ func Add(v1, v2 Value) (Value, error) {
}

lv1, err := newNumeric(v1)
if err != nil {
return NULL, err
}

lv2, err := newNumeric(v2)
if err != nil {
return NULL, err
}

lresult, err := addNumericWithError(lv1, lv2)
if err != nil {
Expand All @@ -61,6 +64,30 @@ func Add(v1, v2 Value) (Value, error) {
return castFromNumeric(lresult, lresult.typ), nil
}

// Subtract takes two values and subtracts them
func Subtract(v1, v2 Value) (Value, error) {
if v1.IsNull() || v2.IsNull() {
return NULL, nil
}

lv1, err := newNumeric(v1)
if err != nil {
return NULL, err
}

lv2, err := newNumeric(v2)
if err != nil {
return NULL, err
}

lresult, err := subtractNumericWithError(lv1, lv2)
if err != nil {
return NULL, err
}

return castFromNumeric(lresult, lresult.typ), nil
}

// NullsafeAdd adds two Values in a null-safe manner. A null value
// is treated as 0. If both values are null, then a null is returned.
// If both values are not null, a numeric value is built
Expand Down Expand Up @@ -243,7 +270,10 @@ func ToInt64(v Value) (int64, error) {

// ToFloat64 converts Value to float64.
func ToFloat64(v Value) (float64, error) {
num, _ := newNumeric(v)
num, err := newNumeric(v)
if err != nil {
return 0, err
}
switch num.typ {
case Int64:
return float64(num.ival), nil
Expand Down Expand Up @@ -376,6 +406,25 @@ func addNumericWithError(v1, v2 numeric) (numeric, error) {

}

func subtractNumericWithError(v1, v2 numeric) (numeric, error) {
v1, v2 = prioritize(v1, v2)
rasikakale marked this conversation as resolved.
Show resolved Hide resolved
switch v1.typ {
case Int64:
return intMinusIntWithError(v1.ival, v2.ival)
case Uint64:
switch v2.typ {
case Int64:
return uintMinusIntWithError(v1.uval, v2.ival)
case Uint64:
return uintMinusUintWithError(v1.uval, v2.uval)
}
case Float64:
return floatMinusAny(v1.fval, v2), nil
}
panic("unreachable")

}

// prioritize reorders the input parameters
// to be Float64, Uint64, Int64.
func prioritize(v1, v2 numeric) (altv1, altv2 numeric) {
Expand All @@ -388,7 +437,6 @@ func prioritize(v1, v2 numeric) (altv1, altv2 numeric) {
if v2.typ == Float64 {
return v2, v1
}

}
return v1, v2
}
Expand All @@ -415,6 +463,15 @@ func intPlusIntWithError(v1, v2 int64) (numeric, error) {
return numeric{typ: Int64, ival: result}, nil
}

func intMinusIntWithError(v1, v2 int64) (numeric, error) {
result := v1 - v2
if v1 > 0 && v2 > math.MaxInt64 || v1 > math.MaxInt64 && v2 > 0 || v1 <= math.MinInt64 && v2 > 0 || v1 > 0 && v2 <= math.MinInt64 {
rasikakale marked this conversation as resolved.
Show resolved Hide resolved
return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT value is out of range in %v - %v", v1, v2)
}

return numeric{typ: Int64, ival: result}, nil
}

func uintPlusInt(v1 uint64, v2 int64) numeric {
return uintPlusUint(v1, uint64(v2))
}
Expand All @@ -424,16 +481,26 @@ func uintPlusIntWithError(v1 uint64, v2 int64) (numeric, error) {
return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT value is out of range in %v + %v", v1, v2)
}

//convert to int -> uint is because for numeric operators (such as + or -)
//where one of the operands is an unsigned integer, the result is unsigned by default.
if v1 >= math.MaxUint64 && v2 > 0 {
return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in %v + %v", v1, v2)
}

// convert to int -> uint is because for numeric operators (such as + or -)
// where one of the operands is an unsigned integer, the result is unsigned by default.
return uintPlusUintWithError(v1, uint64(v2))
}

func uintMinusIntWithError(v1 uint64, v2 int64) (numeric, error) {
if v1 < uint64(v2) {
rasikakale marked this conversation as resolved.
Show resolved Hide resolved
return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in %v - %v", v1, v2)
}
return uintMinusUintWithError(v1, uint64(v2))
rasikakale marked this conversation as resolved.
Show resolved Hide resolved
}

func uintPlusUint(v1, v2 uint64) numeric {
result := v1 + v2
if result < v2 {
return numeric{typ: Float64, fval: float64(v1) + float64(v2)}

}
return numeric{typ: Uint64, uval: result}
}
Expand All @@ -444,6 +511,11 @@ func uintPlusUintWithError(v1, v2 uint64) (numeric, error) {
if result < v2 {
return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in %v + %v", v1, v2)
}
return numeric{typ: Uint64, uval: result}, nil
}

func uintMinusUintWithError(v1, v2 uint64) (numeric, error) {
result := v1 - v2
rasikakale marked this conversation as resolved.
Show resolved Hide resolved

return numeric{typ: Uint64, uval: result}, nil
}
Expand All @@ -458,6 +530,16 @@ func floatPlusAny(v1 float64, v2 numeric) numeric {
return numeric{typ: Float64, fval: v1 + v2.fval}
}

func floatMinusAny(v1 float64, v2 numeric) numeric {
switch v2.typ {
case Int64:
v2.fval = float64(v2.ival)
case Uint64:
v2.fval = float64(v2.uval)
}
return numeric{typ: Float64, fval: v1 - v2.fval}
}

func castFromNumeric(v numeric, resultType querypb.Type) Value {
switch {
case IsSigned(resultType):
Expand Down
132 changes: 119 additions & 13 deletions go/sqltypes/arithmetic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ import (
"vitess.io/vitess/go/vt/vterrors"
)

func TestAdd(t *testing.T) {
func TestSubtract(t *testing.T) {
tcases := []struct {
v1, v2 Value
out Value
err error
}{{

//All Nulls
// All Nulls
v1: NULL,
v2: NULL,
out: NULL,
Expand All @@ -51,24 +50,120 @@ func TestAdd(t *testing.T) {
v2: NewInt32(1),
out: NULL,
}, {
// case with negative value
v1: NewInt64(-1),
v2: NewInt64(-2),
out: NewInt64(1),
}, {
// testing for int64 overflow with min negative value
v1: NewInt64(math.MinInt64),
v2: NewInt64(1),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT value is out of range in -9223372036854775808 - 1"),
}, {
v1: NewUint64(4),
v2: NewInt64(5),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in 4 - 5"),
}, {
v1: NewUint64(7),
v2: NewInt64(5),
out: NewUint64(2),
}, {
v1: NewUint64(math.MaxUint64),
v2: NewInt64(0),
out: NewUint64(math.MaxUint64),
}, {
// testing for int64 overflow
v1: NewInt64(math.MinInt64),
v2: NewUint64(0),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in 0 - -9223372036854775808"),
}, {
v1: TestValue(VarChar, "c"),
v2: NewInt64(1),
out: NewInt64(-1),
}, {
v1: NewUint64(1),
v2: TestValue(VarChar, "c"),
out: NewUint64(1),
}, {
// testing for error for parsing float value to uint64
v1: TestValue(Uint64, "1.2"),
v2: NewInt64(2),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "strconv.ParseUint: parsing \"1.2\": invalid syntax"),
}, {
// testing for error for parsing float value to uint64
v1: NewUint64(2),
v2: TestValue(Uint64, "1.2"),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "strconv.ParseUint: parsing \"1.2\": invalid syntax"),
}, {
// uint64 - uint64
v1: NewUint64(8),
v2: NewUint64(4),
out: NewUint64(4),
}, {
// testing for float subtraction: float - int
v1: NewFloat64(1.2),
v2: NewInt64(2),
out: NewFloat64(-0.8),
}, {
// testing for float subtraction: float - uint
v1: NewFloat64(1.2),
v2: NewUint64(2),
out: NewFloat64(-0.8),
}}

for _, tcase := range tcases {

got, err := Subtract(tcase.v1, tcase.v2)

if !vterrors.Equals(err, tcase.err) {
t.Errorf("Subtract(%v, %v) error: %v, want %v", printValue(tcase.v1), printValue(tcase.v2), vterrors.Print(err), vterrors.Print(tcase.err))
}
if tcase.err != nil {
continue
}

if !reflect.DeepEqual(got, tcase.out) {
t.Errorf("Subtract(%v, %v): %v, want %v", printValue(tcase.v1), printValue(tcase.v2), printValue(got), printValue(tcase.out))
}
}

}

func TestAdd(t *testing.T) {
tcases := []struct {
v1, v2 Value
out Value
err error
}{{
// All Nulls
v1: NULL,
v2: NULL,
out: NULL,
}, {
// First value null.
v1: NewInt32(1),
v2: NULL,
out: NULL,
}, {
// Second value null.
v1: NULL,
v2: NewInt32(1),
out: NULL,
}, {
// case with negatives
v1: NewInt64(-1),
v2: NewInt64(-2),
out: NewInt64(-3),
}, {

// testing for overflow int64
v1: NewInt64(math.MaxInt64),
v2: NewUint64(2),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT value is out of range in 2 + 9223372036854775807"),
}, {

v1: NewInt64(-2),
v2: NewUint64(1),
out: NewUint64(math.MaxUint64),
}, {

v1: NewInt64(math.MaxInt64),
v2: NewInt64(-2),
out: NewInt64(9223372036854775805),
Expand All @@ -83,37 +178,45 @@ func TestAdd(t *testing.T) {
v2: NewUint64(2),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in 18446744073709551615 + 2"),
}, {

// int64 underflow
v1: NewInt64(math.MinInt64),
v2: NewInt64(-2),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT value is out of range in -9223372036854775808 + -2"),
}, {

// checking int64 max value can be returned
v1: NewInt64(math.MaxInt64),
v2: NewUint64(0),
out: NewUint64(9223372036854775807),
}, {

// testing whether uint64 max value can be returned
v1: NewUint64(math.MaxUint64),
v2: NewInt64(0),
out: NewUint64(math.MaxUint64),
}, {

v1: NewUint64(math.MaxInt64),
v2: NewInt64(1),
out: NewUint64(9223372036854775808),
}, {

v1: NewUint64(1),
v2: TestValue(VarChar, "c"),
out: NewUint64(1),
}, {
v1: NewUint64(1),
v2: TestValue(VarChar, "1.2"),
out: NewFloat64(2.2),
}, {
v1: TestValue(Int64, "1.2"),
v2: NewInt64(2),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "strconv.ParseInt: parsing \"1.2\": invalid syntax"),
}, {
v1: NewInt64(2),
v2: TestValue(Int64, "1.2"),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "strconv.ParseInt: parsing \"1.2\": invalid syntax"),
}, {
// testing for uint64 overflow with max uint64 + int value
v1: NewUint64(math.MaxUint64),
v2: NewInt64(2),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in 18446744073709551615 + 2"),
}}

for _, tcase := range tcases {
Expand All @@ -128,7 +231,7 @@ func TestAdd(t *testing.T) {
}

if !reflect.DeepEqual(got, tcase.out) {
t.Errorf("Addition(%v, %v): %v, want %v", printValue(tcase.v1), printValue(tcase.v2), printValue(got), printValue(tcase.out))
t.Errorf("Add(%v, %v): %v, want %v", printValue(tcase.v1), printValue(tcase.v2), printValue(got), printValue(tcase.out))
}
}

Expand Down Expand Up @@ -184,7 +287,7 @@ func TestNullsafeAdd(t *testing.T) {
got := NullsafeAdd(tcase.v1, tcase.v2, Int64)

if !reflect.DeepEqual(got, tcase.out) {
t.Errorf("Add(%v, %v): %v, want %v", printValue(tcase.v1), printValue(tcase.v2), printValue(got), printValue(tcase.out))
t.Errorf("NullsafeAdd(%v, %v): %v, want %v", printValue(tcase.v1), printValue(tcase.v2), printValue(got), printValue(tcase.out))
}
}
}
Expand Down Expand Up @@ -456,6 +559,9 @@ func TestToFloat64(t *testing.T) {
}, {
v: NewFloat64(1.2),
out: 1.2,
}, {
v: TestValue(Int64, "1.2"),
err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "strconv.ParseInt: parsing \"1.2\": invalid syntax"),
}}
for _, tcase := range tcases {
got, err := ToFloat64(tcase.v)
Expand Down