From 9e4ea01ccaee45685e0bdefecfd26ecbf25c5415 Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Tue, 13 Aug 2019 13:44:09 -0700 Subject: [PATCH 01/11] sqltypes: Subtract functionality Signed-off-by: Rasika Kale Signed-off-by: Sugu Sougoumarane --- go/sqltypes/arithmetic.go | 63 +++++++++++++++++++++++-- go/sqltypes/arithmetic_test.go | 85 ++++++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 3 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index f5c18dd43a1..24fe939550d 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -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 { @@ -61,6 +58,24 @@ 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) + + lv2, err := newNumeric(v2) + + 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 @@ -376,6 +391,25 @@ func addNumericWithError(v1, v2 numeric) (numeric, error) { } +func subtractNumericWithError(v1, v2 numeric) (numeric, error) { + v1, v2 = prioritize(v1, v2) + 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 floatPlusAny(v1.fval, v2), nil + } + panic("unreachable") + +} + // prioritize reorders the input parameters // to be Float64, Uint64, Int64. func prioritize(v1, v2 numeric) (altv1, altv2 numeric) { @@ -415,6 +449,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 { + 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)) } @@ -429,6 +472,14 @@ func uintPlusIntWithError(v1 uint64, v2 int64) (numeric, error) { return uintPlusUintWithError(v1, uint64(v2)) } +func uintMinusIntWithError(v1 uint64, v2 int64) (numeric, error) { + if v1 < uint64(v2) { + 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)) +} + func uintPlusUint(v1, v2 uint64) numeric { result := v1 + v2 if result < v2 { @@ -448,6 +499,12 @@ func uintPlusUintWithError(v1, v2 uint64) (numeric, error) { return numeric{typ: Uint64, uval: result}, nil } +func uintMinusUintWithError(v1, v2 uint64) (numeric, error) { + result := v1 - v2 + + return numeric{typ: Uint64, uval: result}, nil +} + func floatPlusAny(v1 float64, v2 numeric) numeric { switch v2.typ { case Int64: diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 81aeba0cf30..f3de7542051 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -29,6 +29,91 @@ import ( "vitess.io/vitess/go/vt/vterrors" ) +func TestSubtract(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 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), + }} + + 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("Subtraction(%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 From 2c30eb41d0186bc412bed97926a44d303107d814 Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Tue, 27 Aug 2019 11:08:25 -0700 Subject: [PATCH 02/11] Fixed code coverage in arithmetic.go - Added more tests in arithmetic_test.go to fix code coverage - Added error checking into arithmetic.go - Implemented function floatMinusAny() for subtraction expression Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic.go | 34 +++++++++++++++++++++++++-- go/sqltypes/arithmetic_test.go | 42 ++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index 24fe939550d..66f7b51b6e2 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -47,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 { @@ -65,8 +71,14 @@ func Subtract(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 := subtractNumericWithError(lv1, lv2) if err != nil { @@ -258,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 @@ -404,7 +419,7 @@ func subtractNumericWithError(v1, v2 numeric) (numeric, error) { return uintMinusUintWithError(v1.uval, v2.uval) } case Float64: - return floatPlusAny(v1.fval, v2), nil + return floatMinusAny(v1.fval, v2), nil } panic("unreachable") @@ -467,6 +482,10 @@ 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) } + 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)) @@ -515,6 +534,17 @@ 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): diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index f3de7542051..381421bf554 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -94,6 +94,31 @@ func TestSubtract(t *testing.T) { v1: NewUint64(1), v2: TestValue(VarChar, "c"), out: NewUint64(1), + }, { + + v1: TestValue(Uint64, "1.2"), + v2: NewInt64(2), + err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "strconv.ParseUint: parsing \"1.2\": invalid syntax"), + }, { + + 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), + }, { + // testin for float subtraction: float - uint + v1: NewFloat64(1.2), + v2: NewUint64(2), + out: NewFloat64(-0.8), }} for _, tcase := range tcases { @@ -199,6 +224,20 @@ func TestAdd(t *testing.T) { 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 { @@ -541,6 +580,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) From 6ab4c17bd926ce4a2511718a9dfd19e1f49ed2ca Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Tue, 27 Aug 2019 11:29:23 -0700 Subject: [PATCH 03/11] Fixed spacing between // and comment Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic.go | 10 +++------- go/sqltypes/arithmetic_test.go | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index 66f7b51b6e2..ef6914467e1 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -64,7 +64,7 @@ func Add(v1, v2 Value) (Value, error) { return castFromNumeric(lresult, lresult.typ), nil } -//Subtract takes two values and subtracts them +// Subtract takes two values and subtracts them func Subtract(v1, v2 Value) (Value, error) { if v1.IsNull() || v2.IsNull() { return NULL, nil @@ -486,8 +486,8 @@ func uintPlusIntWithError(v1 uint64, v2 int64) (numeric, error) { 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. + // 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)) } @@ -495,7 +495,6 @@ func uintMinusIntWithError(v1 uint64, v2 int64) (numeric, error) { if v1 < uint64(v2) { 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)) } @@ -503,7 +502,6 @@ 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} } @@ -514,7 +512,6 @@ 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 } @@ -542,7 +539,6 @@ func floatMinusAny(v1 float64, v2 numeric) numeric { v2.fval = float64(v2.uval) } return numeric{typ: Float64, fval: v1 - v2.fval} - } func castFromNumeric(v numeric, resultType querypb.Type) Value { diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 381421bf554..124cf14636f 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -36,7 +36,7 @@ func TestSubtract(t *testing.T) { err error }{{ - //All Nulls + // All Nulls v1: NULL, v2: NULL, out: NULL, From 2141b6338c95ea9ee8f3b60ae6d933449caad244 Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Tue, 27 Aug 2019 12:50:45 -0700 Subject: [PATCH 04/11] Fixed comment spacing and cleaned up code Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic.go | 1 - go/sqltypes/arithmetic_test.go | 29 ++++------------------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index ef6914467e1..1b1875a579c 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -437,7 +437,6 @@ func prioritize(v1, v2 numeric) (altv1, altv2 numeric) { if v2.typ == Float64 { return v2, v1 } - } return v1, v2 } diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 124cf14636f..98dcb8dbbaf 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -35,58 +35,48 @@ func TestSubtract(t *testing.T) { 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 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), @@ -95,12 +85,12 @@ func TestSubtract(t *testing.T) { 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"), @@ -115,7 +105,7 @@ func TestSubtract(t *testing.T) { v2: NewInt64(2), out: NewFloat64(-0.8), }, { - // testin for float subtraction: float - uint + // testing for float subtraction: float - uint v1: NewFloat64(1.2), v2: NewUint64(2), out: NewFloat64(-0.8), @@ -145,8 +135,7 @@ func TestAdd(t *testing.T) { out Value err error }{{ - - //All Nulls + // All Nulls v1: NULL, v2: NULL, out: NULL, @@ -161,24 +150,20 @@ func TestAdd(t *testing.T) { 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), @@ -193,30 +178,25 @@ 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), @@ -225,7 +205,6 @@ func TestAdd(t *testing.T) { 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"), From 829f3d3891c1bad032768965458ebf61f9d273ab Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Tue, 27 Aug 2019 12:53:23 -0700 Subject: [PATCH 05/11] Fixed error print statements Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 98dcb8dbbaf..4c9e56beddc 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -123,7 +123,7 @@ func TestSubtract(t *testing.T) { } if !reflect.DeepEqual(got, tcase.out) { - t.Errorf("Subtraction(%v, %v): %v, want %v", printValue(tcase.v1), printValue(tcase.v2), printValue(got), printValue(tcase.out)) + t.Errorf("Subtract(%v, %v): %v, want %v", printValue(tcase.v1), printValue(tcase.v2), printValue(got), printValue(tcase.out)) } } @@ -231,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)) } } @@ -287,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)) } } } From 0a1a9afe4803b84c92deacdaa3a03327b2915288 Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Fri, 30 Aug 2019 11:22:47 -0700 Subject: [PATCH 06/11] Fixed arithmetic.go and arithmetic_test.go through comments via github - Added intMinusUintWithError() in arithmetic.go - Fixed error checking in arithmetic.go for intMinusIntWithError() Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic.go | 16 +++++++++++++--- go/sqltypes/arithmetic_test.go | 14 +++++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index 1b1875a579c..6423490be1a 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -407,10 +407,15 @@ func addNumericWithError(v1, v2 numeric) (numeric, error) { } func subtractNumericWithError(v1, v2 numeric) (numeric, error) { - v1, v2 = prioritize(v1, v2) + //v1, v2 = prioritize(v1, v2) switch v1.typ { case Int64: - return intMinusIntWithError(v1.ival, v2.ival) + switch v2.typ { + case Int64: + return intMinusIntWithError(v1.ival, v2.ival) + case Uint64: + return intMinusUintWithError(v1.ival, v2.uval) + } case Uint64: switch v2.typ { case Int64: @@ -465,13 +470,18 @@ func intPlusIntWithError(v1, v2 int64) (numeric, error) { 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 { + + if (result < v1) != (v2 > 0) { 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 intMinusUintWithError(v1 int64, v2 uint64) (numeric, error) { + return intMinusIntWithError(v1, int64(v2)) +} + func uintPlusInt(v1 uint64, v2 int64) numeric { return uintPlusUint(v1, uint64(v2)) } diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 4c9e56beddc..72a0e47206e 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -75,7 +75,7 @@ func TestSubtract(t *testing.T) { // 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"), + out: NewInt64(math.MinInt64), }, { v1: TestValue(VarChar, "c"), v2: NewInt64(1), @@ -109,6 +109,18 @@ func TestSubtract(t *testing.T) { v1: NewFloat64(1.2), v2: NewUint64(2), out: NewFloat64(-0.8), + }, { + v1: NewInt64(-1), + v2: NewUint64(2), + out: NewInt64(-3), + }, { + v1: NewInt64(5), + v2: NewUint64(7), + out: NewInt64(-2), + }, { + v1: NewInt64(-2), + v2: NewInt64(1), + out: NewInt64(-3), }} for _, tcase := range tcases { From 1141f5ae4bba2c8538639fde990eb52d010c0eec Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Fri, 30 Aug 2019 14:54:49 -0700 Subject: [PATCH 07/11] Deleted extra comments and tests in arithmetic.go and arithmetic_test.go Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic.go | 1 - go/sqltypes/arithmetic_test.go | 8 -------- 2 files changed, 9 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index 6423490be1a..f547e25b5c7 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -407,7 +407,6 @@ func addNumericWithError(v1, v2 numeric) (numeric, error) { } func subtractNumericWithError(v1, v2 numeric) (numeric, error) { - //v1, v2 = prioritize(v1, v2) switch v1.typ { case Int64: switch v2.typ { diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 72a0e47206e..da076670a4c 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -113,14 +113,6 @@ func TestSubtract(t *testing.T) { v1: NewInt64(-1), v2: NewUint64(2), out: NewInt64(-3), - }, { - v1: NewInt64(5), - v2: NewUint64(7), - out: NewInt64(-2), - }, { - v1: NewInt64(-2), - v2: NewInt64(1), - out: NewInt64(-3), }} for _, tcase := range tcases { From 667e410506b4174a3dc148c5a6a337954017f35a Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Thu, 5 Sep 2019 11:28:23 -0700 Subject: [PATCH 08/11] Changed Error Checking for subtraction in intMinusUintWithError() - Added test for method in arithmetic_test.go to maximize code coverage Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic.go | 5 ++++- go/sqltypes/arithmetic_test.go | 9 +++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index f547e25b5c7..b4507670c1e 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -478,7 +478,10 @@ func intMinusIntWithError(v1, v2 int64) (numeric, error) { } func intMinusUintWithError(v1 int64, v2 uint64) (numeric, error) { - return intMinusIntWithError(v1, int64(v2)) + if v1 < 0 { + return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in %v - %v", v1, v2) + } + return uintMinusUintWithError(uint64(v1), v2) } func uintPlusInt(v1 uint64, v2 int64) numeric { diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index da076670a4c..2f354b50d2f 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -64,6 +64,7 @@ func TestSubtract(t *testing.T) { v2: NewInt64(5), err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in 4 - 5"), }, { + // testing uint - int v1: NewUint64(7), v2: NewInt64(5), out: NewUint64(2), @@ -75,7 +76,7 @@ func TestSubtract(t *testing.T) { // testing for int64 overflow v1: NewInt64(math.MinInt64), v2: NewUint64(0), - out: NewInt64(math.MinInt64), + err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in -9223372036854775808 - 0"), }, { v1: TestValue(VarChar, "c"), v2: NewInt64(1), @@ -112,7 +113,11 @@ func TestSubtract(t *testing.T) { }, { v1: NewInt64(-1), v2: NewUint64(2), - out: NewInt64(-3), + err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in -1 - 2"), + }, { + v1: NewInt64(2), + v2: NewUint64(1), + out: NewUint64(1), }} for _, tcase := range tcases { From 00b3a085e31f08b148a4b6b72e720e9a1dc1dc97 Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Thu, 5 Sep 2019 14:54:59 -0700 Subject: [PATCH 09/11] Fixed error checking in arithmetic.go under uintPlusIntWithError() - Fixed tests in TestAdd() in arithmetic_test.go Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic.go | 8 +------- go/sqltypes/arithmetic_test.go | 6 +++--- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index b4507670c1e..94488494536 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -19,7 +19,6 @@ package sqltypes import ( "bytes" "fmt" - "math" "strconv" @@ -489,14 +488,9 @@ func uintPlusInt(v1 uint64, v2 int64) numeric { } func uintPlusIntWithError(v1 uint64, v2 int64) (numeric, error) { - if v2 >= math.MaxInt64 && v1 > 0 { - return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT value is out of range in %v + %v", v1, v2) - } - - if v1 >= math.MaxUint64 && v2 > 0 { + if v2 < 0 && v1 < uint64(v2) { 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)) diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 2f354b50d2f..6c99d081d4e 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -164,14 +164,14 @@ func TestAdd(t *testing.T) { v2: NewInt64(-2), out: NewInt64(-3), }, { - // testing for overflow int64 + // testing for overflow int64, result will be unsigned int v1: NewInt64(math.MaxInt64), v2: NewUint64(2), - err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT value is out of range in 2 + 9223372036854775807"), + out: NewUint64(9223372036854775809), }, { v1: NewInt64(-2), v2: NewUint64(1), - out: NewUint64(math.MaxUint64), + err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in 1 + -2"), }, { v1: NewInt64(math.MaxInt64), v2: NewInt64(-2), From 355c915a09b87b440433b8cf5ce6f82c91174a30 Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Tue, 10 Sep 2019 09:51:40 -0700 Subject: [PATCH 10/11] Changed error checking to account for corner cases - checked if v1 < 0 in intMinusUintWithError() - checked if v2 is greater that v1 and greater than 0 in uintMinusIntWithError()_ - within uintMinusIntWithError() checked if v2 < 0, then returned uintPlusIntWithError(v1, -v2) - checked if v2 > v1 in uintMinusUintWithError() - Added function floatMinusAny() in subtractNumericWithError() to account for float - value expressions Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic.go | 30 +++++++++++++++++++++++------- go/sqltypes/arithmetic_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/go/sqltypes/arithmetic.go b/go/sqltypes/arithmetic.go index 94488494536..cde014116eb 100644 --- a/go/sqltypes/arithmetic.go +++ b/go/sqltypes/arithmetic.go @@ -402,7 +402,6 @@ func addNumericWithError(v1, v2 numeric) (numeric, error) { return floatPlusAny(v1.fval, v2), nil } panic("unreachable") - } func subtractNumericWithError(v1, v2 numeric) (numeric, error) { @@ -413,6 +412,8 @@ func subtractNumericWithError(v1, v2 numeric) (numeric, error) { return intMinusIntWithError(v1.ival, v2.ival) case Uint64: return intMinusUintWithError(v1.ival, v2.uval) + case Float64: + return anyMinusFloat(v1, v2.fval), nil } case Uint64: switch v2.typ { @@ -420,12 +421,13 @@ func subtractNumericWithError(v1, v2 numeric) (numeric, error) { return uintMinusIntWithError(v1.uval, v2.ival) case Uint64: return uintMinusUintWithError(v1.uval, v2.uval) + case Float64: + return anyMinusFloat(v1, v2.fval), nil } case Float64: return floatMinusAny(v1.fval, v2), nil } panic("unreachable") - } // prioritize reorders the input parameters @@ -472,12 +474,11 @@ func intMinusIntWithError(v1, v2 int64) (numeric, error) { if (result < v1) != (v2 > 0) { 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 intMinusUintWithError(v1 int64, v2 uint64) (numeric, error) { - if v1 < 0 { + if v1 < 0 || v1 < int64(v2) { return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in %v - %v", v1, v2) } return uintMinusUintWithError(uint64(v1), v2) @@ -497,9 +498,13 @@ func uintPlusIntWithError(v1 uint64, v2 int64) (numeric, error) { } func uintMinusIntWithError(v1 uint64, v2 int64) (numeric, error) { - if v1 < uint64(v2) { + if int64(v1) < v2 && v2 > 0 { return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in %v - %v", v1, v2) } + // uint - (- int) = uint + int + if v2 < 0 { + return uintPlusIntWithError(v1, -v2) + } return uintMinusUintWithError(v1, uint64(v2)) } @@ -513,7 +518,6 @@ func uintPlusUint(v1, v2 uint64) numeric { func uintPlusUintWithError(v1, v2 uint64) (numeric, error) { result := v1 + v2 - if result < v2 { return numeric{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in %v + %v", v1, v2) } @@ -522,7 +526,9 @@ func uintPlusUintWithError(v1, v2 uint64) (numeric, error) { func uintMinusUintWithError(v1, v2 uint64) (numeric, error) { result := v1 - v2 - + if v2 > v1 { + 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 } @@ -546,6 +552,16 @@ func floatMinusAny(v1 float64, v2 numeric) numeric { return numeric{typ: Float64, fval: v1 - v2.fval} } +func anyMinusFloat(v1 numeric, v2 float64) numeric { + switch v1.typ { + case Int64: + v1.fval = float64(v1.ival) + case Uint64: + v1.fval = float64(v1.uval) + } + return numeric{typ: Float64, fval: v1.fval - v2} +} + func castFromNumeric(v numeric, resultType querypb.Type) Value { switch { case IsSigned(resultType): diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 6c99d081d4e..81fc378e198 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -118,6 +118,31 @@ func TestSubtract(t *testing.T) { v1: NewInt64(2), v2: NewUint64(1), out: NewUint64(1), + }, { + // testing int64 - float64 method + v1: NewInt64(-2), + v2: NewFloat64(1.0), + out: NewFloat64(-3.0), + }, { + // testing uint64 - float64 method + v1: NewUint64(1), + v2: NewFloat64(-2.0), + out: NewFloat64(3.0), + }, { + // testing uint - int to return uintplusint + v1: NewUint64(1), + v2: NewInt64(-2), + out: NewUint64(3), + }, { + // testing for float - float + v1: NewFloat64(1.2), + v2: NewFloat64(3.2), + out: NewFloat64(-2), + }, { + // testing uint - uint if v2 > v1 + v1: NewUint64(2), + v2: NewUint64(4), + err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in 2 - 4"), }} for _, tcase := range tcases { From dd2f51f94547d1e2613eef22e99eedc6ff713cd4 Mon Sep 17 00:00:00 2001 From: Rasika Kale Date: Tue, 10 Sep 2019 11:42:09 -0700 Subject: [PATCH 11/11] Added test for uint - (- int) Signed-off-by: Rasika Kale --- go/sqltypes/arithmetic_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/sqltypes/arithmetic_test.go b/go/sqltypes/arithmetic_test.go index 81fc378e198..227b20c7d4c 100644 --- a/go/sqltypes/arithmetic_test.go +++ b/go/sqltypes/arithmetic_test.go @@ -143,6 +143,11 @@ func TestSubtract(t *testing.T) { v1: NewUint64(2), v2: NewUint64(4), err: vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "BIGINT UNSIGNED value is out of range in 2 - 4"), + }, { + // testing uint - (- int) + v1: NewUint64(1), + v2: NewInt64(-2), + out: NewUint64(3), }} for _, tcase := range tcases {