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 77feb76a fix(parquet/pqarrow): reject decimal overflow in integer
columns (#1122)
77feb76a is described below
commit 77feb76a8d2eb25dba8147fc927121262c4603ab
Author: Minh Vu <[email protected]>
AuthorDate: Fri Aug 14 18:39:33 2026 +0200
fix(parquet/pqarrow): reject decimal overflow in integer columns (#1122)
### Rationale for this change
The Parquet decimal integer conversion uses debug assertions for range
checks. Those assertions are disabled in normal builds, so values
outside the signed INT32 or INT64 range can be silently narrowed.
### What changes are included in this PR?
Check signed range boundaries for Decimal128 and Decimal256 values
before converting them to Parquet INT32 or INT64 columns, and return an
error when a value does not fit.
### Are these changes tested?
- `go test ./parquet/pqarrow -run TestDecimalIntegerOverflow`
### Are there any user-facing changes?
Values that do not fit the target Parquet integer type now return an
error instead of being truncated.
---
parquet/pqarrow/decimal_overflow_test.go | 253 +++++++++++++++++++++++++++++++
parquet/pqarrow/encode_arrow.go | 122 +++++++++++++--
2 files changed, 364 insertions(+), 11 deletions(-)
diff --git a/parquet/pqarrow/decimal_overflow_test.go
b/parquet/pqarrow/decimal_overflow_test.go
new file mode 100644
index 00000000..0270f25f
--- /dev/null
+++ b/parquet/pqarrow/decimal_overflow_test.go
@@ -0,0 +1,253 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pqarrow_test
+
+import (
+ "bytes"
+ "context"
+ "math/big"
+ "testing"
+
+ "github.com/apache/arrow-go/v18/arrow"
+ "github.com/apache/arrow-go/v18/arrow/array"
+ "github.com/apache/arrow-go/v18/arrow/decimal128"
+ "github.com/apache/arrow-go/v18/arrow/decimal256"
+ "github.com/apache/arrow-go/v18/arrow/memory"
+ "github.com/apache/arrow-go/v18/parquet"
+ "github.com/apache/arrow-go/v18/parquet/file"
+ "github.com/apache/arrow-go/v18/parquet/internal/encoding"
+ "github.com/apache/arrow-go/v18/parquet/pqarrow"
+ "github.com/apache/arrow-go/v18/parquet/schema"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+func makeDecimalArray(t *testing.T, kind, value string) arrow.Array {
+ t.Helper()
+ n, ok := new(big.Int).SetString(value, 10)
+ require.True(t, ok)
+
+ switch kind {
+ case "decimal128":
+ builder := array.NewDecimal128Builder(memory.DefaultAllocator,
&arrow.Decimal128Type{Precision: 38, Scale: 0})
+ defer builder.Release()
+ builder.Append(decimal128.FromBigInt(n))
+ return builder.NewDecimal128Array()
+ case "decimal256":
+ builder := array.NewDecimal256Builder(memory.DefaultAllocator,
&arrow.Decimal256Type{Precision: 76, Scale: 0})
+ defer builder.Release()
+ builder.Append(decimal256.FromBigInt(n))
+ return builder.NewDecimal256Array()
+ default:
+ t.Fatalf("unknown decimal kind %q", kind)
+ return nil
+ }
+}
+
+func writeDecimalInteger(arr arrow.Array, physical parquet.Type, precision
int32) error {
+ _, err := writeDecimalIntegerData(arr, physical, precision,
parquet.Repetitions.Required)
+ return err
+}
+
+func writeDecimalIntegerData(arr arrow.Array, physical parquet.Type, precision
int32, repetition parquet.Repetition) ([]byte, error) {
+ mem := memory.DefaultAllocator
+ primitive := schema.Must(schema.NewPrimitiveNodeLogical(
+ "value",
+ repetition,
+ schema.NewDecimalLogicalType(precision, 0),
+ physical,
+ -1,
+ -1,
+ ))
+ parquetSchema := schema.MustGroup(schema.NewGroupNode(
+ "schema",
+ parquet.Repetitions.Required,
+ schema.FieldList{primitive},
+ -1,
+ ))
+
+ sink := encoding.NewBufferWriter(0, mem)
+ defer sink.Release()
+ writer := file.NewParquetWriter(sink, parquetSchema)
+ defer writer.Close()
+
+ rowGroup, err := writer.AppendRowGroupChecked()
+ if err != nil {
+ return nil, err
+ }
+ defer rowGroup.Close()
+
+ column, err := rowGroup.NextColumn()
+ if err != nil {
+ return nil, err
+ }
+ defer column.Close()
+
+ var defLevels []int16
+ leafFieldNullable := false
+ if repetition == parquet.Repetitions.Optional {
+ defLevels = make([]int16, arr.Len())
+ leafFieldNullable = true
+ for i := range defLevels {
+ if !arr.IsNull(i) {
+ defLevels[i] = 1
+ }
+ }
+ }
+
+ if err := pqarrow.WriteArrowToColumn(
+ pqarrow.NewArrowWriteContext(context.Background(), nil),
+ column,
+ arr,
+ defLevels,
+ nil,
+ leafFieldNullable,
+ ); err != nil {
+ return nil, err
+ }
+ if err := column.Close(); err != nil {
+ return nil, err
+ }
+ if err := rowGroup.Close(); err != nil {
+ return nil, err
+ }
+ if err := writer.Close(); err != nil {
+ return nil, err
+ }
+ return append([]byte(nil), sink.Bytes()...), nil
+}
+
+func TestDecimalIntegerOverflow(t *testing.T) {
+ tests := []struct {
+ name string
+ kind string
+ value string
+ physical parquet.Type
+ precision int32
+ wantErr bool
+ }{
+ {name: "decimal128 int32 precision max", kind: "decimal128",
value: "999999999", physical: parquet.Types.Int32, precision: 9},
+ {name: "decimal128 int32 negative precision max", kind:
"decimal128", value: "-999999999", physical: parquet.Types.Int32, precision: 9},
+ {name: "decimal128 int32 precision overflow", kind:
"decimal128", value: "1000000000", physical: parquet.Types.Int32, precision: 9,
wantErr: true},
+ {name: "decimal128 int32 negative precision overflow", kind:
"decimal128", value: "-1000000000", physical: parquet.Types.Int32, precision:
9, wantErr: true},
+ {name: "decimal256 int32 precision max", kind: "decimal256",
value: "999999999", physical: parquet.Types.Int32, precision: 9},
+ {name: "decimal256 int32 negative precision max", kind:
"decimal256", value: "-999999999", physical: parquet.Types.Int32, precision: 9},
+ {name: "decimal128 int32 overflow", kind: "decimal128", value:
"2147483648", physical: parquet.Types.Int32, precision: 9, wantErr: true},
+ {name: "decimal256 int32 overflow", kind: "decimal256", value:
"-2147483649", physical: parquet.Types.Int32, precision: 9, wantErr: true},
+ {name: "decimal128 int64 precision max", kind: "decimal128",
value: "999999999999999999", physical: parquet.Types.Int64, precision: 18},
+ {name: "decimal128 int64 negative precision max", kind:
"decimal128", value: "-999999999999999999", physical: parquet.Types.Int64,
precision: 18},
+ {name: "decimal128 int64 precision overflow", kind:
"decimal128", value: "1000000000000000000", physical: parquet.Types.Int64,
precision: 18, wantErr: true},
+ {name: "decimal128 int64 negative precision overflow", kind:
"decimal128", value: "-1000000000000000000", physical: parquet.Types.Int64,
precision: 18, wantErr: true},
+ {name: "decimal256 int64 precision max", kind: "decimal256",
value: "999999999999999999", physical: parquet.Types.Int64, precision: 18},
+ {name: "decimal256 int64 negative precision max", kind:
"decimal256", value: "-999999999999999999", physical: parquet.Types.Int64,
precision: 18},
+ {name: "decimal128 int64 physical max outside precision", kind:
"decimal128", value: "9223372036854775807", physical: parquet.Types.Int64,
precision: 18, wantErr: true},
+ {name: "decimal128 int64 overflow", kind: "decimal128", value:
"9223372036854775808", physical: parquet.Types.Int64, precision: 18, wantErr:
true},
+ {name: "decimal256 int64 overflow", kind: "decimal256", value:
"-9223372036854775809", physical: parquet.Types.Int64, precision: 18, wantErr:
true},
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ arr := makeDecimalArray(t, tc.kind, tc.value)
+ defer arr.Release()
+
+ err := writeDecimalInteger(arr, tc.physical,
tc.precision)
+ if tc.wantErr {
+ require.ErrorIs(t, err, arrow.ErrInvalid)
+ } else {
+ require.NoError(t, err)
+ }
+ })
+ }
+}
+
+func makeNullableDecimalArray(t *testing.T, kind, invalid string) arrow.Array {
+ t.Helper()
+ invalidValue, ok := new(big.Int).SetString(invalid, 10)
+ require.True(t, ok)
+
+ switch kind {
+ case "decimal128":
+ builder := array.NewDecimal128Builder(memory.DefaultAllocator,
&arrow.Decimal128Type{Precision: 38, Scale: 0})
+ defer builder.Release()
+ builder.AppendNull()
+ builder.Append(decimal128.FromI64(42))
+ arr := builder.NewDecimal128Array()
+ arr.Values()[0] = decimal128.FromBigInt(invalidValue)
+ return arr
+ case "decimal256":
+ builder := array.NewDecimal256Builder(memory.DefaultAllocator,
&arrow.Decimal256Type{Precision: 76, Scale: 0})
+ defer builder.Release()
+ builder.AppendNull()
+ builder.Append(decimal256.FromI64(42))
+ arr := builder.NewDecimal256Array()
+ arr.Values()[0] = decimal256.FromBigInt(invalidValue)
+ return arr
+ default:
+ t.Fatalf("unknown decimal kind %q", kind)
+ return nil
+ }
+}
+
+func TestDecimalIntegerNullBackingValues(t *testing.T) {
+ tests := []struct {
+ name string
+ kind string
+ physical parquet.Type
+ precision int32
+ invalid string
+ }{
+ {name: "decimal128 to int32", kind: "decimal128", physical:
parquet.Types.Int32, precision: 9, invalid: "2147483648"},
+ {name: "decimal256 to int32", kind: "decimal256", physical:
parquet.Types.Int32, precision: 9, invalid: "2147483648"},
+ {name: "decimal128 to int64", kind: "decimal128", physical:
parquet.Types.Int64, precision: 18, invalid: "9223372036854775808"},
+ {name: "decimal256 to int64", kind: "decimal256", physical:
parquet.Types.Int64, precision: 18, invalid: "9223372036854775808"},
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ arr := makeNullableDecimalArray(t, tc.kind, tc.invalid)
+ defer arr.Release()
+
+ _, err := writeDecimalIntegerData(arr, tc.physical,
tc.precision, parquet.Repetitions.Optional)
+ require.NoError(t, err)
+ })
+ }
+}
+
+func TestDecimalIntegerNullRoundTrip(t *testing.T) {
+ arr := makeNullableDecimalArray(t, "decimal128", "2147483648")
+ defer arr.Release()
+
+ data, err := writeDecimalIntegerData(arr, parquet.Types.Int32, 9,
parquet.Repetitions.Optional)
+ require.NoError(t, err)
+
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(t, 0)
+ reader, err := file.NewParquetReader(bytes.NewReader(data),
file.WithReadProps(parquet.NewReaderProperties(mem)))
+ require.NoError(t, err)
+ defer reader.Close()
+ arrowReader, err := pqarrow.NewFileReader(reader,
pqarrow.ArrowReadProperties{}, mem)
+ require.NoError(t, err)
+ tbl, err := arrowReader.ReadTable(context.Background())
+ require.NoError(t, err)
+ defer tbl.Release()
+
+ result, ok := tbl.Column(0).Data().Chunk(0).(*array.Decimal128)
+ require.True(t, ok)
+ require.Len(t, result.Values(), 2)
+ assert.True(t, result.IsNull(0))
+ assert.Equal(t, decimal128.FromI64(42), result.Value(1))
+}
diff --git a/parquet/pqarrow/encode_arrow.go b/parquet/pqarrow/encode_arrow.go
index 73b52a34..b71452f9 100644
--- a/parquet/pqarrow/encode_arrow.go
+++ b/parquet/pqarrow/encode_arrow.go
@@ -35,7 +35,7 @@ import (
"github.com/apache/arrow-go/v18/internal/utils"
"github.com/apache/arrow-go/v18/parquet"
"github.com/apache/arrow-go/v18/parquet/file"
- "github.com/apache/arrow-go/v18/parquet/internal/debug"
+ "github.com/apache/arrow-go/v18/parquet/schema"
)
// get the count of the number of leaf arrays for the type
@@ -66,6 +66,68 @@ func nullableRoot(manifest *SchemaManifest, field
*SchemaField) bool {
return nullable
}
+func decimal128FitsInt32(val decimal128.Num) bool {
+ switch val.HighBits() {
+ case 0:
+ return val.LowBits() <= math.MaxInt32
+ case -1:
+ return val.LowBits() >= uint64(0xffffffff80000000)
+ default:
+ return false
+ }
+}
+
+func decimal128FitsInt64(val decimal128.Num) bool {
+ switch val.HighBits() {
+ case 0:
+ return val.LowBits() <= math.MaxInt64
+ case -1:
+ return val.LowBits() >= uint64(0x8000000000000000)
+ default:
+ return false
+ }
+}
+
+func decimal256FitsInt32(val decimal256.Num) bool {
+ words := val.Array()
+ switch {
+ case words[1] == 0 && words[2] == 0 && words[3] == 0:
+ return words[0] <= math.MaxInt32
+ case words[1] == math.MaxUint64 && words[2] == math.MaxUint64 &&
words[3] == math.MaxUint64:
+ return words[0] >= uint64(0xffffffff80000000)
+ default:
+ return false
+ }
+}
+
+func decimal256FitsInt64(val decimal256.Num) bool {
+ words := val.Array()
+ switch {
+ case words[1] == 0 && words[2] == 0 && words[3] == 0:
+ return words[0] <= math.MaxInt64
+ case words[1] == math.MaxUint64 && words[2] == math.MaxUint64 &&
words[3] == math.MaxUint64:
+ return words[0] >= uint64(0x8000000000000000)
+ default:
+ return false
+ }
+}
+
+func targetDecimalPrecision(cw file.ColumnChunkWriter) (int32, bool) {
+ logical, ok := cw.Descr().LogicalType().(schema.DecimalLogicalType)
+ if !ok {
+ return 0, false
+ }
+ return logical.Precision(), true
+}
+
+func decimal128FitsTargetPrecision(val decimal128.Num, precision int32) bool {
+ return precision > 0 && precision <= decimal128.MaxPrecision &&
val.FitsInPrecision(precision)
+}
+
+func decimal256FitsTargetPrecision(val decimal256.Num, precision int32) bool {
+ return precision > 0 && precision <= decimal256.MaxPrecision &&
val.FitsInPrecision(precision)
+}
+
// arrowColumnWriter is a convenience object for easily writing arrow data to
a specific
// set of columns in a parquet file. Since a single arrow array can itself be
a nested type
// consisting of multiple columns of data, this will write to all of the
appropriate leaves in
@@ -350,15 +412,33 @@ func writeDenseArrow(ctx *arrowWriteContext, cw
file.ColumnChunkWriter, leafArr
data[idx] = int32(val / 86400000) //
coerce date64 values
}
case arrow.DECIMAL128:
- for idx, val := range
leafArr.(*array.Decimal128).Values() {
- debug.Assert(val.HighBits() == 0 ||
val.HighBits() == -1, "casting Decimal128 greater than the value range; high
bits must be 0 or -1")
- debug.Assert(int64(val.LowBits()) <=
math.MaxUint32, "casting Decimal128 to int32 when value > MaxUint32")
+ arr := leafArr.(*array.Decimal128)
+ precision, hasPrecision :=
targetDecimalPrecision(cw)
+ for idx, val := range arr.Values() {
+ if arr.IsNull(idx) {
+ continue
+ }
+ if !decimal128FitsInt32(val) {
+ return fmt.Errorf("%w:
Decimal128 value at index %d does not fit in Parquet INT32", arrow.ErrInvalid,
idx)
+ }
+ if hasPrecision &&
!decimal128FitsTargetPrecision(val, precision) {
+ return fmt.Errorf("%w:
Decimal128 value at index %d does not fit Parquet DECIMAL precision %d",
arrow.ErrInvalid, idx, precision)
+ }
data[idx] = int32(val.LowBits())
}
case arrow.DECIMAL256:
- for idx, val := range
leafArr.(*array.Decimal256).Values() {
- debug.Assert(val.Array()[3] == 0 ||
val.Array()[3] == 0xFFFFFFFF, "casting Decimal128 greater than the value range;
high bits must be 0 or -1")
- debug.Assert(val.LowBits() <=
math.MaxUint32, "casting Decimal128 to int32 when value > MaxUint32")
+ arr := leafArr.(*array.Decimal256)
+ precision, hasPrecision :=
targetDecimalPrecision(cw)
+ for idx, val := range arr.Values() {
+ if arr.IsNull(idx) {
+ continue
+ }
+ if !decimal256FitsInt32(val) {
+ return fmt.Errorf("%w:
Decimal256 value at index %d does not fit in Parquet INT32", arrow.ErrInvalid,
idx)
+ }
+ if hasPrecision &&
!decimal256FitsTargetPrecision(val, precision) {
+ return fmt.Errorf("%w:
Decimal256 value at index %d does not fit Parquet DECIMAL precision %d",
arrow.ErrInvalid, idx, precision)
+ }
data[idx] = int32(val.LowBits())
}
default:
@@ -433,15 +513,35 @@ func writeDenseArrow(ctx *arrowWriteContext, cw
file.ColumnChunkWriter, leafArr
case arrow.DECIMAL128:
ctx.dataBuffer.ResizeNoShrink(arrow.Int64Traits.BytesRequired(leafArr.Len()))
data =
arrow.Int64Traits.CastFromBytes(ctx.dataBuffer.Bytes())
- for idx, val := range
leafArr.(*array.Decimal128).Values() {
- debug.Assert(val.HighBits() == 0 ||
val.HighBits() == -1, "trying to cast Decimal128 to int64 greater than range,
high bits must be 0 or -1")
+ arr := leafArr.(*array.Decimal128)
+ precision, hasPrecision := targetDecimalPrecision(cw)
+ for idx, val := range arr.Values() {
+ if arr.IsNull(idx) {
+ continue
+ }
+ if !decimal128FitsInt64(val) {
+ return fmt.Errorf("%w: Decimal128 value
at index %d does not fit in Parquet INT64", arrow.ErrInvalid, idx)
+ }
+ if hasPrecision &&
!decimal128FitsTargetPrecision(val, precision) {
+ return fmt.Errorf("%w: Decimal128 value
at index %d does not fit Parquet DECIMAL precision %d", arrow.ErrInvalid, idx,
precision)
+ }
data[idx] = int64(val.LowBits())
}
case arrow.DECIMAL256:
ctx.dataBuffer.ResizeNoShrink(arrow.Int64Traits.BytesRequired(leafArr.Len()))
data =
arrow.Int64Traits.CastFromBytes(ctx.dataBuffer.Bytes())
- for idx, val := range
leafArr.(*array.Decimal256).Values() {
- debug.Assert(val.Array()[3] == 0 ||
val.Array()[3] == 0xFFFFFFFF, "trying to cast Decimal128 to int64 greater than
range, high bits must be 0 or -1")
+ arr := leafArr.(*array.Decimal256)
+ precision, hasPrecision := targetDecimalPrecision(cw)
+ for idx, val := range arr.Values() {
+ if arr.IsNull(idx) {
+ continue
+ }
+ if !decimal256FitsInt64(val) {
+ return fmt.Errorf("%w: Decimal256 value
at index %d does not fit in Parquet INT64", arrow.ErrInvalid, idx)
+ }
+ if hasPrecision &&
!decimal256FitsTargetPrecision(val, precision) {
+ return fmt.Errorf("%w: Decimal256 value
at index %d does not fit Parquet DECIMAL precision %d", arrow.ErrInvalid, idx,
precision)
+ }
data[idx] = int64(val.LowBits())
}
default:
