Copilot commented on code in PR #1515:
URL: https://github.com/apache/arrow-go/pull/1515#discussion_r4530696716
##########
arrow/decimal128/decimal128.go:
##########
@@ -282,14 +283,35 @@ func FromString(v string, prec, scale int32) (n Num, err
error) {
return Num{}, errors.New("bitlen too large for
decimal128")
}
n = FromBigInt(val)
- n, _ = n.Div(scaleMultipliers[-scale])
+ if scale >= -MaxScale {
+ n, _ = n.Div(scaleMultipliers[-scale])
+ } else {
+ // |n| < 2^127 < 10^39, so the quotient is zero.
+ n = Num{}
+ }
} else {
// 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.
- p := (&big.Float{}).SetInt(scaleMultipliers[scale].BigInt())
- out.SetPrec(precInBits).Mul(out, p)
+ if scale <= MaxScale {
+ p :=
(&big.Float{}).SetInt(scaleMultipliers[scale].BigInt())
+ out.SetPrec(precInBits).Mul(out, p)
+ } else {
+ pow10.Mul(out.SetPrec(precInBits), int64(scale), 128+64)
Review Comment:
The out-of-table path misrounds exact halfway values:
`decimal128.FromString("5e-41", 38, 40)` returns coefficient 0 instead of 1,
with the symmetric error for negative input. The same issue occurs in
`arrow/decimal256/decimal256.go:177`: `FromString("5e-93", 76, 92)` returns 0
instead of 1. Resolve ties using the original decimal coefficient/exponent
rather than the rounded binary approximation, retain the bounded-allocation
guards, and add positive and negative halfway regression cases for both types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
