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-go.git
The following commit(s) were added to refs/heads/main by this push:
new d577b68a fix(arrow/decimal): widen Decimal64 rescale to 64 bits (#1307)
d577b68a is described below
commit d577b68a4f0330b4c9c42889023736f72883ff91
Author: Madan Kumar <[email protected]>
AuthorDate: Wed Sep 16 00:28:26 2026 +0530
fix(arrow/decimal): widen Decimal64 rescale to 64 bits (#1307)
### Rationale for this change
`Decimal64.rescaleWouldCauseDataLoss` (arrow/decimal/decimal.go) is a
copy of the `Decimal32` version that was never widened to 64 bits — it
still uses `bits.Div32` / `bits.Mul32` with `uint32(n)` /
`uint32(multiplier)`. Any `Decimal64` whose magnitude exceeds `uint32`
max (~4.29e9, well inside Decimal64's 18-digit range) is truncated to 32
bits before the multiply/divide, so `Rescale` returns a corrupt value
and/or a spurious `rescale data loss` error.
Concretely, on `main`:
```go
decimal.Decimal64(10_000_000_000).Rescale(2, 1)
// returns (141006540, error "rescale data loss")
// correct: (1000000000, nil) // 1e10 / 10 = 1e9, exact
```
### What changes are included in this PR?
- Use `bits.Div64` / `bits.Mul64` over `uint64` in
`Decimal64.rescaleWouldCauseDataLoss` (the `Decimal32` version is
already correct for 32 bits).
- `TestDecimalRescale` had baked in the truncated behavior: it asserted
that `Decimal64(555555).Rescale(0, 5)` returns `rescale data loss`, but
`555555` at scale 5 is `55,555,500,000`, which fits in `Decimal64` and
is lossless. Corrected that assertion, checked the exact result, and
added a value that genuinely overflows `Decimal64` (`5e18` rescaled up)
so the real data-loss path stays covered.
RED→GREEN verified: the updated `TestDecimalRescale` fails on the
unpatched code and passes with the fix; the full `arrow/decimal` package
suite is green; `gofmt`/`go vet`/`golangci-lint` clean.
### Are these changes tested?
Yes — `arrow/decimal/decimal_test.go`.
### Are there any user-facing changes?
`Decimal64.Rescale` now returns correct results (and no spurious error)
for values above ~4.29e9. No API change.
Signed-off-by: Madan Kumar <[email protected]>
---
arrow/decimal/decimal.go | 4 ++--
arrow/decimal/decimal_test.go | 9 ++++++++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/arrow/decimal/decimal.go b/arrow/decimal/decimal.go
index 1a1f00da..4d6563cc 100644
--- a/arrow/decimal/decimal.go
+++ b/arrow/decimal/decimal.go
@@ -419,11 +419,11 @@ func (n Decimal32) rescaleWouldCauseDataLoss(deltaScale
int32, multiplier Decima
func (n Decimal64) rescaleWouldCauseDataLoss(deltaScale int32, multiplier
Decimal64) (out Decimal64, loss bool) {
if deltaScale < 0 {
debug.Assert(multiplier != 0, "multiplier must not be zero")
- quo, remainder := bits.Div32(0, uint32(n), uint32(multiplier))
+ quo, remainder := bits.Div64(0, uint64(n), uint64(multiplier))
return Decimal64(quo), remainder != 0
}
- overflow, result := bits.Mul32(uint32(n), uint32(multiplier))
+ overflow, result := bits.Mul64(uint64(n), uint64(multiplier))
if overflow != 0 {
return Decimal64(result), true
}
diff --git a/arrow/decimal/decimal_test.go b/arrow/decimal/decimal_test.go
index 9b4f04f5..55635762 100644
--- a/arrow/decimal/decimal_test.go
+++ b/arrow/decimal/decimal_test.go
@@ -386,7 +386,14 @@ func TestDecimalRescale(t *testing.T) {
_, err = decimal.Decimal32(555555).Rescale(0, 5)
assert.ErrorContains(t, err, "rescale data loss")
- _, err = decimal.Decimal64(555555).Rescale(0, 5)
+ // 555555 at scale 5 is 55,555,500,000, which fits comfortably in
+ // Decimal64's range, so the rescale is lossless. (The 32-bit path
wrongly
+ // reported data loss by truncating the value to uint32 before
multiplying.)
+ out64, err := decimal.Decimal64(555555).Rescale(0, 5)
+ assert.NoError(t, err)
+ assert.Equal(t, decimal.Decimal64(55_555_500_000), out64)
+ // A value that genuinely overflows Decimal64 still reports data loss.
+ _, err = decimal.Decimal64(5_000_000_000_000_000_000).Rescale(0, 1)
assert.ErrorContains(t, err, "rescale data loss")
}
