From 809a02d9869be904f0ecb614c5293301368b8405 Mon Sep 17 00:00:00 2001 From: Travis Date: Wed, 22 Apr 2020 17:01:09 -0500 Subject: [PATCH 1/2] Adjust decimal precision if we have decimal places to sacrifice. --- pql/decimal.go | 52 +++++++++++++++++++++++++++++++++++++++++++++ pql/decimal_test.go | 14 ++++++++++-- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/pql/decimal.go b/pql/decimal.go index 08a4be5c2..68c2704f4 100644 --- a/pql/decimal.go +++ b/pql/decimal.go @@ -396,6 +396,17 @@ func ParseDecimal(s string) (Decimal, error) { scale = 0 } + // If the mantissa can't be represented by an int64, but it contains + // enough decimal places such that we can sacrifice precision, then + // we do that. This is an attempt to be compatible with the way + // `strconv.ParseFloat` works. + if m, s, ok := reducePrecision(sign, mantissa, scale); ok { + mantissa = m + scale = s + } else { + return Decimal{}, errors.Errorf("value out of range: %s", mantissa) + } + // We have to use ParseUint here (as opposed to ParseInt) because // math.MinInt64 is a valid value, but its absolute value is not. // So this allows us to handle that one value without overflow, and @@ -420,6 +431,47 @@ func ParseDecimal(s string) (Decimal, error) { }, nil } +// reducePrecision takes a []byte mantissa and scale, and if possible +// will adjust the mantissa (by reducing precision) until it can be +// represented by an int64. The returned bool indicates whether the +// reduction was successful. +func reducePrecision(sign bool, mantissa []byte, scale int64) ([]byte, int64, bool) { + // Trim leading zeros before considering length. + var zeroIdx int + for i := range mantissa { + if mantissa[i] == '0' { + zeroIdx++ + } else { + break + } + } + mantissa = mantissa[zeroIdx:] + + // If we zero out the mantissa to an empty + // string, that means it's value should be 0. + if len(mantissa) == 0 { + mantissa = []byte{'0'} + } + + lenMantissa := len(mantissa) + maxStr := "9223372036854775807" + if sign { + maxStr = "9223372036854775808" + } + + if lenMantissa <= 18 || (lenMantissa == 19 && string(mantissa) <= maxStr) { + return mantissa, scale, true + } + + // If we don't have any decimal places to sacrifice, + // we can't change anything. + if scale <= 0 { + return mantissa, scale, false + } + + return reducePrecision(sign, mantissa[:len(mantissa)-1], scale-1) +} + func quotient(d Decimal) int64 { if d.Scale == 0 { return d.Value diff --git a/pql/decimal_test.go b/pql/decimal_test.go index 84a781b70..b5b77fc79 100644 --- a/pql/decimal_test.go +++ b/pql/decimal_test.go @@ -67,6 +67,15 @@ func TestDecimal(t *testing.T) { {"9223372036854775807000", pql.Decimal{9223372036854775807, -3}, ""}, {"-9223372036854775807000", pql.Decimal{-9223372036854775807, -3}, ""}, + // precision adjustment + {"2.666666666666666667", pql.Decimal{2666666666666666667, 18}, ""}, + {"2.6666666666666666667", pql.Decimal{2666666666666666666, 18}, ""}, + {"2.6666666666666666666667", pql.Decimal{2666666666666666666, 18}, ""}, + {"-9.223372036854775808", pql.Decimal{-9223372036854775808, 18}, ""}, + {"-9.223372036854775809", pql.Decimal{-922337203685477580, 17}, ""}, + {"9.223372036854775807", pql.Decimal{9223372036854775807, 18}, ""}, + {"9.223372036854775808", pql.Decimal{922337203685477580, 17}, ""}, + // Error cases. {"", pql.Decimal{}, "decimal string is empty"}, {"-", pql.Decimal{}, "decimal string is empty"}, @@ -75,8 +84,9 @@ func TestDecimal(t *testing.T) { {"0.12.3", pql.Decimal{}, "invalid decimal string"}, {"--12300", pql.Decimal{}, "invalid syntax"}, - {"922337203685477580.9", pql.Decimal{}, "value out of range"}, - {"-922337203685477580.9", pql.Decimal{}, "value out of range"}, + // These are no longer error cases since we introduced precision adjustment. + //{"922337203685477580.9", pql.Decimal{}, "value out of range"}, + //{"-922337203685477580.9", pql.Decimal{}, "value out of range"}, {"9223372036854775808000", pql.Decimal{}, "value out of range"}, {"-9223372036854775809000", pql.Decimal{}, "value out of range"}, } From 432e18d5718ddc075187483ccb5b251bf7290c4a Mon Sep 17 00:00:00 2001 From: Travis Date: Wed, 22 Apr 2020 19:11:03 -0500 Subject: [PATCH 2/2] return early on mantissa=0 --- pql/decimal.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pql/decimal.go b/pql/decimal.go index 68c2704f4..c6a7f834c 100644 --- a/pql/decimal.go +++ b/pql/decimal.go @@ -451,6 +451,7 @@ func reducePrecision(sign bool, mantissa []byte, scale int64) ([]byte, int64, bo // string, that means it's value should be 0. if len(mantissa) == 0 { mantissa = []byte{'0'} + return mantissa, scale, true } lenMantissa := len(mantissa)