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 8100cd75 fix(arrow/decimal): handle native word widths in big integer
conversions (#1409)
8100cd75 is described below
commit 8100cd75076d8fe4ea5ae31190534d1212a7a5c5
Author: Matt Topol <[email protected]>
AuthorDate: Mon Oct 26 15:48:31 2026 +0000
fix(arrow/decimal): handle native word widths in big integer conversions
(#1409)
### Rationale for this change
Found while fixing the 32-bit test failures in #1408.
Decimal conversions are wrong on 32-bit platforms. `decimal128.Num` and
`decimal256.Num` use 64-bit limbs, but their `big.Int` conversions
assumed `big.Word` is also 64 bits. On 386 it is 32 bits, so:
- `decimal128.FromBigInt` read `b[0]`/`b[1]` as the low and high limbs
(`decimal128.go:95–97`), misplacing words and dropping the rest;
- `decimal128`'s `toBigIntPositive` built `[]big.Word{big.Word(n.lo),
big.Word(n.hi)}` (`decimal128.go:362`), truncating each limb;
- `decimal256.FromBigInt` copied word *i* into limb *i*
(`decimal256.go:370`), misplacing words and panicking when a value
needed more than four native words;
- `decimal256`'s `toBigIntPositive` truncated its limbs the same way
(`decimal256.go:379`).
Anything going through `big.Int` (string formatting and parsing,
rescaling, `GetOneForMarshal`, CSV decimal parsing) produced wrong
values or spurious precision errors on 386, e.g. `TestDecimal128Slice`
printed `-1717986918` instead of `-7.378697629e+18`, and
`TestDecimal128GetOneForMarshal` turned `1234567890.123456789` into
`437278697.08398250845211745659132109676`.
The run-end-encoded validation fix that was originally part of this PR
has moved to #1421.
### What changes are included in this PR?
- **`arrow/decimal128`, `arrow/decimal256`:** when `bits.UintSize ==
32`, combine pairs of 32-bit `big.Word`s into each 64-bit limb in
`FromBigInt`, and split each limb into two words in `toBigIntPositive`.
`bits.UintSize` is a constant, so 64-bit builds compile the existing
code unchanged. Also corrects the `decimal128.FromBigInt` doc (it panics
above 127 bits, as the code already does).
- **CI:** the 386 job also runs the decimal packages' tests.
### Are these changes tested?
Yes.
- **On `GOARCH=386`:** `TestDecimal128Slice`,
`TestDecimal128StringRoundTrip`, `TestDecimal128GetOneForMarshal`,
`TestDecimal256Slice`, `TestDecimal256GetOneForMarshal`, all nine
`TestDecimal/TestWithNulls#01` precisions,
`TestDecimalScalarStringCastUsesExactScaling` and CSV's
`TestReadCSVDecimalCols` all failed before and pass now. The
`TestDecimalToBigInt` tests in both packages gain word-boundary cases
with independently computed expectations, which fail on 386 without the
fix (decimal256 panics). `arrow/decimal128`, `arrow/decimal256` and
`arrow/decimal/...` pass in full on 386.
- **amd64:** `arrow/decimal128`, `arrow/decimal256`, `arrow/decimal/...`
pass.
- `go vet` and `gofmt -l` are clean on the changed packages.
### Are there any user-facing changes?
Yes: decimal values convert correctly on 32-bit platforms. No API
changes.
---
.github/workflows/test.yml | 2 +-
arrow/decimal128/decimal128.go | 28 +++++++++++++++++++++-------
arrow/decimal128/decimal128_test.go | 11 +++++++++++
arrow/decimal256/decimal256.go | 20 ++++++++++++++++++--
arrow/decimal256/decimal256_test.go | 16 ++++++++++++++++
5 files changed, 67 insertions(+), 10 deletions(-)
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 1e8985d4..c8801ec9 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -317,7 +317,7 @@ jobs:
run: GOARCH=386 go build ./...
- name: Test
# WIP refactor, only tests in the specified dirs have been fixed
- run: GOARCH=386 go test ./parquet/file/...
+ run: GOARCH=386 go test ./parquet/file/... ./arrow/decimal/...
./arrow/decimal128/... ./arrow/decimal256/...
tinygo:
name: TinyGo
runs-on: ubuntu-latest
diff --git a/arrow/decimal128/decimal128.go b/arrow/decimal128/decimal128.go
index fca8c799..2febecea 100644
--- a/arrow/decimal128/decimal128.go
+++ b/arrow/decimal128/decimal128.go
@@ -75,7 +75,7 @@ func FromI64(v int64) Num {
}
// FromBigInt will convert a big.Int to a Num, if the value in v has a
-// BitLen > 128, this will panic.
+// BitLen > 127, this will panic.
func FromBigInt(v *big.Int) (n Num) {
bitlen := v.BitLen()
if bitlen > 127 {
@@ -86,14 +86,22 @@ func FromBigInt(v *big.Int) (n Num) {
return
}
- // if the value is negative, then get the high and low bytes from
- // v, and then negate it. this is because Num uses a two's compliment
- // representation of values and big.Int stores the value as a bool for
- // the sign and the absolute value of the integer. This means that the
- // raw bytes are *always* the absolute value.
+ // big.Int stores the magnitude in native-sized words with a separate
sign,
+ // while Num uses two's complement with 64-bit limbs. On 32-bit
platforms,
+ // combine pairs of words before applying the sign.
b := v.Bits()
n.lo = uint64(b[0])
- if len(b) > 1 {
+ if bits.UintSize == 32 {
+ if len(b) > 1 {
+ n.lo |= uint64(b[1]) << 32
+ }
+ if len(b) > 2 {
+ n.hi = int64(b[2])
+ }
+ if len(b) > 3 {
+ n.hi |= int64(b[3]) << 32
+ }
+ } else if len(b) > 1 {
n.hi = int64(b[1])
}
if v.Sign() < 0 {
@@ -359,6 +367,12 @@ func (n Num) Sign() int {
}
func toBigIntPositive(n Num) *big.Int {
+ if bits.UintSize == 32 {
+ return (&big.Int{}).SetBits([]big.Word{
+ big.Word(n.lo), big.Word(n.lo >> 32),
+ big.Word(n.hi), big.Word(n.hi >> 32),
+ })
+ }
return (&big.Int{}).SetBits([]big.Word{big.Word(n.lo), big.Word(n.hi)})
}
diff --git a/arrow/decimal128/decimal128_test.go
b/arrow/decimal128/decimal128_test.go
index ba5539f1..95da30ae 100644
--- a/arrow/decimal128/decimal128_test.go
+++ b/arrow/decimal128/decimal128_test.go
@@ -374,6 +374,13 @@ func TestDecimalToBigInt(t *testing.T) {
{-18446744073709552, 7083549724304524577,
"-340282366920938463463374607431711455"},
{1, 4611686018427387904, "23058430092136939520"},
{0, 0, "0"},
+ {0, 1, "1"},
+ {0, math.MaxUint32, "4294967295"},
+ {0, 1 << 32, "4294967296"},
+ {0, math.MaxUint64, "18446744073709551615"},
+ {1, 0, "18446744073709551616"},
+ {1 << 32, 0, "79228162514264337593543950336"},
+ {math.MaxInt64, math.MaxUint64,
"170141183460469231731687303715884105727"},
}
for _, tc := range tests {
t.Run("", func(t *testing.T) {
@@ -384,6 +391,10 @@ func TestDecimalToBigInt(t *testing.T) {
n2 := decimal128.FromBigInt(bi)
assert.Equal(t, n.LowBits(), n2.LowBits())
assert.Equal(t, n.HighBits(), n2.HighBits())
+
+ bi, ok := new(big.Int).SetString(tc.exp, 10)
+ require.True(t, ok)
+ assert.Equal(t, n, decimal128.FromBigInt(bi))
})
}
}
diff --git a/arrow/decimal256/decimal256.go b/arrow/decimal256/decimal256.go
index a473505f..0b45d2e7 100644
--- a/arrow/decimal256/decimal256.go
+++ b/arrow/decimal256/decimal256.go
@@ -357,6 +357,7 @@ func (n Num) Sign() int {
return int(1 | (int64(n.arr[3]) >> 63))
}
+// FromBigInt converts a big.Int to a Num. It panics if v.BitLen() > 255.
func FromBigInt(v *big.Int) (n Num) {
bitlen := v.BitLen()
if bitlen > 255 {
@@ -366,8 +367,15 @@ func FromBigInt(v *big.Int) (n Num) {
}
b := v.Bits()
- for i, bits := range b {
- n.arr[i] = uint64(bits)
+ if bits.UintSize == 32 {
+ // Each 64-bit limb contains two native-sized big.Int words.
+ for i, word := range b {
+ n.arr[i/2] |= uint64(word) << (32 * (i % 2))
+ }
+ } else {
+ for i, word := range b {
+ n.arr[i] = uint64(word)
+ }
}
if v.Sign() < 0 {
return n.Negate()
@@ -376,6 +384,14 @@ func FromBigInt(v *big.Int) (n Num) {
}
func toBigIntPositive(n Num) *big.Int {
+ if bits.UintSize == 32 {
+ return new(big.Int).SetBits([]big.Word{
+ big.Word(n.arr[0]), big.Word(n.arr[0] >> 32),
+ big.Word(n.arr[1]), big.Word(n.arr[1] >> 32),
+ big.Word(n.arr[2]), big.Word(n.arr[2] >> 32),
+ big.Word(n.arr[3]), big.Word(n.arr[3] >> 32),
+ })
+ }
return new(big.Int).SetBits([]big.Word{big.Word(n.arr[0]),
big.Word(n.arr[1]), big.Word(n.arr[2]), big.Word(n.arr[3])})
}
diff --git a/arrow/decimal256/decimal256_test.go
b/arrow/decimal256/decimal256_test.go
index 0d1f14f2..5112cb4a 100644
--- a/arrow/decimal256/decimal256_test.go
+++ b/arrow/decimal256/decimal256_test.go
@@ -25,6 +25,7 @@ import (
"github.com/apache/arrow-go/v18/arrow/decimal256"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func TestFromU64(t *testing.T) {
@@ -412,6 +413,17 @@ func TestDecimalToBigInt(t *testing.T) {
{[4]uint64{0, 0, 0, 0}, "0"},
{[4]uint64{17877984925544397504, 5352188884907840935,
234631617561833724, 196678011949953713},
"1234567890123456789012345678901234567890123456789012345678901234567890123456"},
{[4]uint64{568759148165154112, 13094555188801710680,
18212112456147717891, 18250066061759597902},
"-1234567890123456789012345678901234567890123456789012345678901234567890123456"},
+ {[4]uint64{1}, "1"},
+ {[4]uint64{math.MaxUint32}, "4294967295"},
+ {[4]uint64{1 << 32}, "4294967296"},
+ {[4]uint64{math.MaxUint64}, "18446744073709551615"},
+ {[4]uint64{0, 1}, "18446744073709551616"},
+ {[4]uint64{0, 1 << 32}, "79228162514264337593543950336"},
+ {[4]uint64{0, 0, 1}, "340282366920938463463374607431768211456"},
+ {[4]uint64{0, 0, 1 << 32},
"1461501637330902918203684832716283019655932542976"},
+ {[4]uint64{0, 0, 0, 1},
"6277101735386680763835789423207666416102355444464034512896"},
+ {[4]uint64{0, 0, 0, 1 << 32},
"26959946667150639794667015087019630673637144422540572481103610249216"},
+ {[4]uint64{math.MaxUint64, math.MaxUint64, math.MaxUint64,
math.MaxInt64},
"57896044618658097711785492504343953926634992332820282019728792003956564819967"},
}
for _, tc := range tests {
t.Run("", func(t *testing.T) {
@@ -421,6 +433,10 @@ func TestDecimalToBigInt(t *testing.T) {
assert.Equal(t, tc.exp, bi.String())
n2 := decimal256.FromBigInt(bi)
assert.Equal(t, n2.Array(), n.Array())
+
+ bi, ok := new(big.Int).SetString(tc.exp, 10)
+ require.True(t, ok)
+ assert.Equal(t, n, decimal256.FromBigInt(bi))
})
}
}
