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/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new b7a385a0 fix(literals): decimal literal marshalbinary (#745)
b7a385a0 is described below
commit b7a385a06e18bb4792d0f3d7c2245d0b4ebe6bed
Author: Matt Topol <[email protected]>
AuthorDate: Thu Feb 19 21:11:42 2026 -0500
fix(literals): decimal literal marshalbinary (#745)
fixes #731
---
literals.go | 66 ++++++++++++++++++++++++++++++++++++++++++++++++--------
literals_test.go | 36 +++++++++++++++++++++++++++++++
2 files changed, 93 insertions(+), 9 deletions(-)
diff --git a/literals.go b/literals.go
index a50f155e..28bcacc5 100644
--- a/literals.go
+++ b/literals.go
@@ -1299,15 +1299,56 @@ func (d DecimalLiteral) MarshalBinary() (data []byte,
err error) {
// stored as unscaled value in two's compliment big-endian values
// using the minimum number of bytes for the values
n := decimal128.Num(d.Val).BigInt()
- minBytes := (n.BitLen() + 8) / 8
- // bytes gives absolute value as big-endian bytes
- data = n.FillBytes(make([]byte, minBytes))
- if n.Sign() < 0 {
- // convert to 2's complement for negative value
- for i, v := range data {
- data[i] = ^v
+
+ if n.Sign() >= 0 {
+ // For non-negative values, use standard encoding
+ minBytes := (n.BitLen() + 8) / 8
+ if minBytes == 0 {
+ minBytes = 1
}
- data[len(data)-1] += 1
+ data = n.FillBytes(make([]byte, minBytes))
+
+ return data, err
+ }
+
+ // For negative values, we need to compute minBytes differently
+ // to handle byte boundaries correctly
+ bitLen := n.BitLen()
+ minBytes := (bitLen + 7) / 8
+
+ // When BitLen is exactly at a byte boundary (multiple of 8),
+ // we need to check if we need an extra byte for the sign bit
+ if bitLen%8 == 0 {
+ absVal := new(big.Int).Abs(n)
+ // If |n| is exactly 2^(bitLen-1), it fits perfectly in
bitLen/8 bytes
+ // Examples: -128 (2^7), -32768 (2^15), -8388608 (2^23)
+ powerOf2 := new(big.Int).Lsh(big.NewInt(1), uint(bitLen-1))
+ if absVal.Cmp(powerOf2) == 0 {
+ minBytes = bitLen / 8
+ } else {
+ // BitLen is at boundary but value isn't exactly
2^(bitLen-1)
+ // Examples: -255, -32767
+ // We need an extra byte to ensure the sign bit is set
+ minBytes++
+ }
+ }
+
+ if minBytes == 0 {
+ minBytes = 1
+ }
+
+ // Convert to two's complement using proper carry propagation
+ data = n.FillBytes(make([]byte, minBytes))
+ // Invert all bits
+ for i := range data {
+ data[i] = ^data[i]
+ }
+ // Add 1 with proper carry propagation
+ carry := byte(1)
+ for i := len(data) - 1; i >= 0 && carry > 0; i-- {
+ sum := uint16(data[i]) + uint16(carry)
+ data[i] = byte(sum)
+ carry = byte(sum >> 8)
}
return data, err
@@ -1334,7 +1375,14 @@ func (d *DecimalLiteral) UnmarshalBinary(data []byte)
error {
for i, b := range data {
out[i] = ^b
}
- out[len(out)-1] += 1
+
+ // Add 1 with proper carry propagation
+ carry := byte(1)
+ for i := len(out) - 1; i >= 0 && carry > 0; i-- {
+ sum := uint16(out[i]) + uint16(carry)
+ out[i] = byte(sum)
+ carry = byte(sum >> 8)
+ }
value := (&big.Int{}).SetBytes(out)
d.Val = decimal128.FromBigInt(value.Neg(value))
diff --git a/literals_test.go b/literals_test.go
index d90170cb..3ba9f396 100644
--- a/literals_test.go
+++ b/literals_test.go
@@ -1308,3 +1308,39 @@ func TestDecimalMaxMinRoundTrip(t *testing.T) {
})
}
}
+
+func TestDecimalMarshalBinaryIssue731(t *testing.T) {
+ tests := []struct {
+ name string
+ value int64
+ expected []byte
+ }{
+ // Issue 1: Lost carry for -256
+ {"lost carry -256", -256, []byte{0xff, 0x00}},
+ // Issue 2: Wrong minBytes for powers of 2
+ {"-128 (-2^7)", -128, []byte{0x80}},
+ {"-32768 (-2^15)", -32768, []byte{0x80, 0x00}},
+ {"-8388608 (-2^23)", -8388608, []byte{0x80, 0x00, 0x00}},
+ // Additional edge cases
+ {"-127 (not power of 2)", -127, []byte{0x81}},
+ {"-255", -255, []byte{0xff, 0x01}},
+ {"-257", -257, []byte{0xfe, 0xff}},
+ {"-32767", -32767, []byte{0x80, 0x01}},
+ {"-1", -1, []byte{0xff}},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ dec := iceberg.DecimalLiteral{Val:
decimal128.FromI64(tt.value), Scale: 0}
+ data, err := dec.MarshalBinary()
+ require.NoError(t, err)
+ assert.Equal(t, tt.expected, data, "value: %d",
tt.value)
+
+ // Round-trip test
+ var decoded iceberg.DecimalLiteral
+ err = decoded.UnmarshalBinary(data)
+ require.NoError(t, err)
+ assert.True(t, dec.Equals(decoded), "round-trip failed
for %d: got %v", tt.value, decoded)
+ })
+ }
+}
