From 9d278d4bea8070617e4bb6fd5da3481c7f17c57c Mon Sep 17 00:00:00 2001 From: jennifersp Date: Mon, 27 Feb 2023 15:38:33 -0800 Subject: [PATCH 1/2] fix division edge cases --- enginetest/queries/queries.go | 4 ++++ enginetest/queries/script_queries.go | 17 ++++++++++++++ sql/expression/div.go | 35 ++++++++++++++++++++++++---- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/enginetest/queries/queries.go b/enginetest/queries/queries.go index 11188faf90..d3116b317a 100644 --- a/enginetest/queries/queries.go +++ b/enginetest/queries/queries.go @@ -3444,6 +3444,10 @@ var QueryTests = []QueryTest{ Query: "select -1<<'a';", Expected: []sql.Row{{uint64(18446744073709551615)}}, }, + { + Query: "select -1.00 div 2;", + Expected: []sql.Row{{0}}, + }, { Query: "select 'a' div 'a';", Expected: []sql.Row{{nil}}, diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go index fd9af02a9d..d70bb41fcc 100644 --- a/enginetest/queries/script_queries.go +++ b/enginetest/queries/script_queries.go @@ -2823,6 +2823,23 @@ var ScriptTests = []ScriptTest{ }, }, }, + { + Name: "division and int division operation on negative, small and big value for decimal type column of table", + SetUpScript: []string{ + "create table t (d decimal(25,10) primary key);", + "insert into t values (-4990), (2), (22336578);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "select d div 314990 from t order by d;", + Expected: []sql.Row{{0}, {0}, {70}}, + }, + { + Query: "select d / 314990 from t order by d;", + Expected: []sql.Row{{"-0.01584177275469"}, {"0.00000634940792"}, {"70.91202260389219"}}, + }, + }, + }, } var SpatialScriptTests = []ScriptTest{ diff --git a/sql/expression/div.go b/sql/expression/div.go index e8609e9cd8..f7438acfd7 100644 --- a/sql/expression/div.go +++ b/sql/expression/div.go @@ -17,6 +17,7 @@ package expression import ( "fmt" "math" + "strconv" "strings" "time" @@ -174,6 +175,21 @@ func (d *Div) evalLeftRight(ctx *sql.Context, row sql.Row) (interface{}, interfa return nil, nil, err } + // this operation is only done on the left value as the scale/fraction part of the leftmost value + // is used to calculate the scale of the final result. If the value is GetField of decimal type column + // the decimal value evaluated does not always match the scale of column type definition + if dt, ok := d.Left.Type().(sql.DecimalType); ok { + if dVal, ok := lval.(decimal.Decimal); ok { + ts := int32(dt.Scale()) + if ts > dVal.Exponent()*-1 { + lval, err = decimal.NewFromString(dVal.StringFixed(ts)) + if err != nil { + return nil, nil, err + } + } + } + } + rval, err = d.Right.Eval(ctx, row) if err != nil { return nil, nil, err @@ -255,7 +271,7 @@ func (d *Div) div(ctx *sql.Context, lval, rval interface{}) (interface{}, error) d.curIntermediatePrecisionInc += int(math.Ceil(float64(r.Exponent()*-1) / float64(divIntermediatePrecisionInc))) } - storedScale := int32(d.curIntermediatePrecisionInc * divIntermediatePrecisionInc) + storedScale := d.leftmostScale + int32(d.curIntermediatePrecisionInc*divIntermediatePrecisionInc) l = l.Truncate(storedScale) r = r.Truncate(storedScale) @@ -410,6 +426,15 @@ func getScaleOfLeftmostValue(ctx *sql.Context, row sql.Row, e sql.Expression, d, return 0 } _, s := GetPrecisionAndScale(lval) + // the leftmost value can be row value of decimal type column + // the evaluated value does not always match the scale of column type definition + typ := a.Left.Type() + if dt, dok := typ.(sql.DecimalType); dok { + ts := dt.Scale() + if ts > s { + s = ts + } + } return int32(s) } else { return getScaleOfLeftmostValue(ctx, row, a.Left, d, dScale) @@ -717,9 +742,11 @@ func intDiv(ctx *sql.Context, lval, rval interface{}) (interface{}, error) { // intDiv operation gets the integer part of the divided value divRes := l.DivRound(r, 2) - intPart := divRes.IntPart() - - if (divRes.IsNegative() && intPart >= 0) || (divRes.IsPositive() && intPart < 0) { + // cannot use IntPart() function of decimal.Decimal package as it returns 0 as undefined value for out of range value + // it causes valid result value of 0 to be the same as invalid out of range value of 0. The fraction part + // should not be rounded, so truncate the result wih 0 precision. + intPart, err := strconv.ParseInt(divRes.Truncate(0).String(), 10, 64) + if err != nil { return nil, ErrIntDivDataOutOfRange.New(l.StringFixed(l.Exponent()), r.StringFixed(r.Exponent())) } From fc1b435376d62d53cc0d6ae2947185d68b7ef234 Mon Sep 17 00:00:00 2001 From: jennifersp Date: Tue, 28 Feb 2023 12:30:54 -0800 Subject: [PATCH 2/2] add comment --- sql/expression/div.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sql/expression/div.go b/sql/expression/div.go index f7438acfd7..4817eb6cb7 100644 --- a/sql/expression/div.go +++ b/sql/expression/div.go @@ -740,12 +740,14 @@ func intDiv(ctx *sql.Context, lval, rval interface{}) (interface{}, error) { return nil, nil } - // intDiv operation gets the integer part of the divided value - divRes := l.DivRound(r, 2) + // intDiv operation gets the integer part of the divided value without rounding the result with 0 precision + // We get division result with non-zero precision and then truncate it to get integer part without it being rounded + divRes := l.DivRound(r, 2).Truncate(0) + // cannot use IntPart() function of decimal.Decimal package as it returns 0 as undefined value for out of range value // it causes valid result value of 0 to be the same as invalid out of range value of 0. The fraction part // should not be rounded, so truncate the result wih 0 precision. - intPart, err := strconv.ParseInt(divRes.Truncate(0).String(), 10, 64) + intPart, err := strconv.ParseInt(divRes.String(), 10, 64) if err != nil { return nil, ErrIntDivDataOutOfRange.New(l.StringFixed(l.Exponent()), r.StringFixed(r.Exponent())) }