From c533af4b020a5647c6f46b211f181991642062a2 Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Fri, 23 Oct 2026 12:29:06 -0400 Subject: [PATCH 1/2] fix(arrow/decimal): handle native word widths in big integer conversions Decimal128 and Decimal256 treated math/big.Word as a 64-bit limb. On 32-bit platforms this truncated and misplaced coefficient bits, and Decimal256.FromBigInt could index past its four-limb array. Combine and split pairs of 32-bit words while preserving the native 64-bit conversion paths. Cover each word boundary and independently check FromBigInt against literal expected coefficients. Generated-by: OpenAI Codex (GPT-6 Astra) --- arrow/decimal128/decimal128.go | 28 +++++++++++++++++++++------- arrow/decimal128/decimal128_test.go | 11 +++++++++++ arrow/decimal256/decimal256.go | 20 ++++++++++++++++++-- arrow/decimal256/decimal256_test.go | 16 ++++++++++++++++ 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/arrow/decimal128/decimal128.go b/arrow/decimal128/decimal128.go index fca8c799f..2febecea4 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 ba5539f1c..95da30ae1 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 a473505f1..0b45d2e7a 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 0d1f14f20..5112cb4a3 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)) }) } } From 458cf8254b0d1f33d370b00c893feaf3dafd80bc Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Fri, 23 Oct 2026 12:41:56 -0400 Subject: [PATCH 2/2] ci: run decimal tests in the 386 job The decimal128/decimal256 big.Int conversion fix has 32-bit-only code paths (bits.UintSize == 32), but the 386 job only ran parquet/file tests, so CI never exercised them. The decimal packages now pass on 386; add them to the job. --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1e8985d48..c8801ec9f 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