winklemad opened a new pull request, #1307: URL: https://github.com/apache/arrow-go/pull/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. -- 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]
