This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 966a804015 GH-35310: [Go] Incorrect value decimal128 from string
(#35311)
966a804015 is described below
commit 966a8040151ebe6276d14ee0c6e98b085daab887
Author: Matt Topol <[email protected]>
AuthorDate: Mon Apr 24 20:36:06 2023 -0400
GH-35310: [Go] Incorrect value decimal128 from string (#35311)
### Rationale for this change
Fix edge case of rounding when parsing a decimal128 value from string with
a high precision.
### What changes are included in this PR?
Adding a simple 0.5 rounding based on the signbit before we truncate the
value to an integer.
### Are these changes tested?
Yes, an edge case test case was added.
### Are there any user-facing changes?
Some values that were previously incorrect will now be correct.
* Closes: #35310
Authored-by: Matt Topol <[email protected]>
Signed-off-by: Matt Topol <[email protected]>
---
go/arrow/decimal128/decimal128.go | 15 ++++++++++++++-
go/arrow/decimal128/decimal128_test.go | 4 +++-
go/arrow/decimal256/decimal256_test.go | 4 +++-
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/go/arrow/decimal128/decimal128.go
b/go/arrow/decimal128/decimal128.go
index 1a471ca9e6..aa66783b14 100644
--- a/go/arrow/decimal128/decimal128.go
+++ b/go/arrow/decimal128/decimal128.go
@@ -224,6 +224,8 @@ func FromFloat64(v float64, prec, scale int32) (Num, error)
{
return fromPositiveFloat64(v, prec, scale)
}
+var pt5 = big.NewFloat(0.5)
+
func FromString(v string, prec, scale int32) (n Num, err error) {
// time for some math!
// Our input precision means "number of digits of precision" but the
@@ -259,8 +261,19 @@ func FromString(v string, prec, scale int32) (n Num, err
error) {
return
}
+ // Since we're going to truncate this to get an integer, we need to
round
+ // the value instead because of edge cases so that we match how other
implementations
+ // (e.g. C++) handles Decimal values. So if we're negative we'll
subtract 0.5 and if
+ // we're positive we'll add 0.5.
+ out.Mul(out, big.NewFloat(math.Pow10(int(scale)))).SetPrec(precInBits)
+ if out.Signbit() {
+ out.Sub(out, pt5)
+ } else {
+ out.Add(out, pt5)
+ }
+
var tmp big.Int
- val, _ := out.Mul(out,
big.NewFloat(math.Pow10(int(scale)))).SetPrec(precInBits).Int(&tmp)
+ val, _ := out.Int(&tmp)
if val.BitLen() > 127 {
return Num{}, errors.New("bitlen too large for decimal128")
}
diff --git a/go/arrow/decimal128/decimal128_test.go
b/go/arrow/decimal128/decimal128_test.go
index 2e9865c45f..75b8522aea 100644
--- a/go/arrow/decimal128/decimal128_test.go
+++ b/go/arrow/decimal128/decimal128_test.go
@@ -657,11 +657,13 @@ func TestFromString(t *testing.T) {
{"1e1", 10, 0},
{"+234.567", 234567, 3},
{"1e-37", 1, 37},
+ {"2112.33", 211233, 2},
+ {"-2112.33", -211233, 2},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("%s_%d", tt.s, tt.expectedScale), func(t
*testing.T) {
- n, err := decimal128.FromString(tt.s, 8,
tt.expectedScale)
+ n, err := decimal128.FromString(tt.s, 37,
tt.expectedScale)
assert.NoError(t, err)
ex := decimal128.FromI64(tt.expected)
diff --git a/go/arrow/decimal256/decimal256_test.go
b/go/arrow/decimal256/decimal256_test.go
index 7a0f7a2e99..233d840110 100644
--- a/go/arrow/decimal256/decimal256_test.go
+++ b/go/arrow/decimal256/decimal256_test.go
@@ -561,11 +561,13 @@ func TestFromString(t *testing.T) {
{"1e1", 10, 0},
{"+234.567", 234567, 3},
{"1e-37", 1, 37},
+ {"2112.33", 211233, 2},
+ {"-2112.33", -211233, 2},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("%s_%d", tt.s, tt.expectedScale), func(t
*testing.T) {
- n, err := decimal256.FromString(tt.s, 8,
tt.expectedScale)
+ n, err := decimal256.FromString(tt.s, 35,
tt.expectedScale)
assert.NoError(t, err)
ex := decimal256.FromI64(tt.expected)
