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 d98c04cc fix(parquet): read null fixed-size-list parents (#1099)
d98c04cc is described below
commit d98c04cc2246228f94a3cee43c88c23fed82e402
Author: Minh Vu <[email protected]>
AuthorDate: Mon Aug 10 20:42:12 2026 +0200
fix(parquet): read null fixed-size-list parents (#1099)
### Rationale for this change
Parquet does not store child values for a null fixed-size-list parent,
while Arrow requires list_size child slots for every parent. The
existing reader assumes every parent consumes exactly list_size values
and does not validate the final span.
### What changes are included in this PR?
Reconstruct children from valid and null parent runs:
- validate that each valid parent consumes exactly list_size values
- validate that null parents consume no stored child values
- materialize list_size null children for each null parent
- validate the final parent span
When there are no null parents, validate the spans and reuse the decoded
child array directly.
### Are these changes tested?
- `go test ./parquet/pqarrow -run 'TestParquetArrowIO/TestFixedSizeList'
-count=1`
### Are there any user-facing changes?
No API changes. This corrects the reported behavior while preserving the
existing ownership and compatibility contracts.
---
parquet/pqarrow/column_readers.go | 114 ++++++++++++++++++++++++++-------
parquet/pqarrow/column_readers_test.go | 110 +++++++++++++++++++++++++++++++
parquet/pqarrow/encode_arrow_test.go | 33 ++++++++++
3 files changed, 234 insertions(+), 23 deletions(-)
diff --git a/parquet/pqarrow/column_readers.go
b/parquet/pqarrow/column_readers.go
index 9064d018..5aedeb47 100644
--- a/parquet/pqarrow/column_readers.go
+++ b/parquet/pqarrow/column_readers.go
@@ -453,6 +453,10 @@ func (lr *listReader) LoadBatch(nrecords int64) error {
}
func (lr *listReader) BuildArray(lenBound int64) (*arrow.Chunked, error) {
+ return lr.buildArray(lenBound)
+}
+
+func (lr *listReader) buildArray(lenBound int64) (*arrow.Chunked, error) {
var (
defLevels []int16
repLevels []int16
@@ -523,6 +527,14 @@ func (lr *listReader) BuildArray(lenBound int64)
(*arrow.Chunked, error) {
return nil, err
}
defer item.Release()
+ itemArr := array.MakeFromData(item)
+ defer itemArr.Release()
+
+ if lr.field.Type.ID() == arrow.FIXED_SIZE_LIST {
+ offsetData :=
arrow.Int32Traits.CastFromBytes(offsetsBuffer.Bytes())
+ return lr.buildFixedSizeListArray(int(validityIO.Read),
offsetData, validityBuffer,
+ validityIO.NullCount, itemArr)
+ }
buffers := []*memory.Buffer{nil, offsetsBuffer}
if validityIO.NullCount > 0 {
@@ -531,38 +543,94 @@ func (lr *listReader) BuildArray(lenBound int64)
(*arrow.Chunked, error) {
data := array.NewData(lr.field.Type, int(validityIO.Read), buffers,
[]arrow.ArrayData{item}, int(validityIO.NullCount), 0)
defer data.Release()
- if lr.field.Type.ID() == arrow.FIXED_SIZE_LIST {
- defer data.Buffers()[1].Release()
- offsetData :=
arrow.Int32Traits.CastFromBytes(offsetsBuffer.Bytes())
- listSize := lr.field.Type.(*arrow.FixedSizeListType).Len()
- for x := 1; x < data.Len(); x++ {
- size := offsetData[x] - offsetData[x-1]
- if size != listSize {
- return nil, fmt.Errorf("expected all lists to
be of size=%d, but index %d had size=%d", listSize, x, size)
+ out := array.MakeFromData(data)
+ defer out.Release()
+ return arrow.NewChunked(lr.field.Type, []arrow.Array{out}), nil
+}
+
+func (lr *listReader) buildFixedSizeListArray(length int, offsets []int32,
validityBuffer *memory.Buffer,
+ nullCount int64, item arrow.Array) (*arrow.Chunked, error) {
+ listType := lr.field.Type.(*arrow.FixedSizeListType)
+ listSize := int(listType.Len())
+
+ if offsets[0] != 0 {
+ return nil, fmt.Errorf("fixed-size list first offset must be
zero, got %d", offsets[0])
+ }
+ if int64(offsets[length]) != int64(item.Len()) {
+ return nil, fmt.Errorf("fixed-size list final offset %d does
not match decoded child length %d",
+ offsets[length], item.Len())
+ }
+
+ if nullCount == 0 {
+ for idx := 0; idx < length; idx++ {
+ if size := offsets[idx+1] - offsets[idx]; size !=
int32(listSize) {
+ return nil, fmt.Errorf("expected all lists to
be of size=%d, but index %d had size=%d", listSize, idx, size)
+ }
+ }
+ data := array.NewData(lr.field.Type, length,
[]*memory.Buffer{nil},
+ []arrow.ArrayData{item.Data()}, 0, 0)
+ defer data.Release()
+ out := array.MakeFromData(data)
+ defer out.Release()
+ return arrow.NewChunked(lr.field.Type, []arrow.Array{out}), nil
+ }
+
+ // Each validity run becomes one piece. Alternating valid and null
parents
+ // create O(length) temporary arrays before concatenation.
+ pieces := make([]arrow.Array, 0, length)
+ defer func() { releaseArrays(pieces) }()
+
+ for i := 0; i < length; {
+ valid := !lr.field.Nullable ||
bitutil.BitIsSet(validityBuffer.Bytes(), i)
+ end := i + 1
+ for end < length {
+ nextValid := !lr.field.Nullable ||
bitutil.BitIsSet(validityBuffer.Bytes(), end)
+ if nextValid != valid {
+ break
+ }
+ end++
+ }
+
+ if valid {
+ for idx := i; idx < end; idx++ {
+ if size := offsets[idx+1] - offsets[idx]; size
!= int32(listSize) {
+ return nil, fmt.Errorf("expected all
lists to be of size=%d, but index %d had size=%d", listSize, idx, size)
+ }
}
+ pieces = append(pieces, array.NewSlice(item,
int64(offsets[i]), int64(offsets[end])))
+ } else {
+ for idx := i; idx < end; idx++ {
+ if size := offsets[idx+1] - offsets[idx]; size
!= 0 {
+ return nil, fmt.Errorf("null fixed-size
list at index %d consumed %d child values", idx, size)
+ }
+ }
+ pieces = append(pieces,
array.MakeArrayOfNull(lr.rctx.mem, listType.Elem(), (end-i)*listSize))
}
- data.Buffers()[1] = nil
+ i = end
+ }
+
+ if len(pieces) == 0 {
+ pieces = append(pieces, array.MakeArrayOfNull(lr.rctx.mem,
listType.Elem(), 0))
+ }
+ child, err := array.Concatenate(pieces, lr.rctx.mem)
+ if err != nil {
+ return nil, err
+ }
+ defer child.Release()
+
+ buffers := []*memory.Buffer{nil}
+ if nullCount > 0 {
+ buffers[0] = validityBuffer
}
+ data := array.NewData(lr.field.Type, length, buffers,
[]arrow.ArrayData{child.Data()}, int(nullCount), 0)
+ defer data.Release()
out := array.MakeFromData(data)
defer out.Release()
return arrow.NewChunked(lr.field.Type, []arrow.Array{out}), nil
}
-// column reader logic for fixed size lists instead of variable length ones.
-type fixedSizeListReader struct {
- *listReader
-}
-
func newFixedSizeListReader(rctx *readerCtx, field *arrow.Field, info
file.LevelInfo, childRdr *ColumnReader, props ArrowReadProperties)
*ColumnReader {
- childRdr.Retain()
- lr := listReader{rctx: rctx, field: field, info: info, itemRdr:
childRdr, props: props}
- lr.refCount.Add(1)
-
- return &ColumnReader{
- &fixedSizeListReader{
- &lr,
- },
- }
+ return newListReader(rctx, field, info, childRdr, props)
}
// helper function to combine chunks into a single array.
diff --git a/parquet/pqarrow/column_readers_test.go
b/parquet/pqarrow/column_readers_test.go
index 6a86b3f9..1371d6f9 100644
--- a/parquet/pqarrow/column_readers_test.go
+++ b/parquet/pqarrow/column_readers_test.go
@@ -24,6 +24,7 @@ import (
"github.com/apache/arrow-go/v18/arrow"
"github.com/apache/arrow-go/v18/arrow/array"
+ "github.com/apache/arrow-go/v18/arrow/extensions"
"github.com/apache/arrow-go/v18/arrow/memory"
"github.com/apache/arrow-go/v18/parquet"
"github.com/apache/arrow-go/v18/parquet/compress"
@@ -156,6 +157,115 @@ func TestChunksToSingle(t *testing.T) {
})
}
+func TestBuildFixedSizeListArrayRequiresExactChildSpan(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(t, 0)
+
+ tests := []struct {
+ name string
+ length int
+ offsets []int32
+ nullCount int64
+ validity byte
+ itemLen int
+ errContains string
+ }{
+ {name: "nonzero first offset", length: 1, offsets: []int32{1,
3}, itemLen: 3,
+ errContains: "first offset must be zero"},
+ {name: "trailing child values", length: 1, offsets: []int32{0,
2}, itemLen: 3,
+ errContains: "final offset 2 does not match decoded
child length 3"},
+ {name: "final offset beyond child values", length: 1, offsets:
[]int32{0, 3}, itemLen: 2,
+ errContains: "final offset 3 does not match decoded
child length 2"},
+ {name: "nullable trailing child values", length: 2, offsets:
[]int32{0, 2, 2}, nullCount: 1, validity: 0x01, itemLen: 3,
+ errContains: "final offset 2 does not match decoded
child length 3"},
+ {name: "valid parent with wrong child span", length: 2,
offsets: []int32{0, 1, 2}, itemLen: 2,
+ errContains: "index 0 had size=1"},
+ {name: "null parent consumes child values", length: 2, offsets:
[]int32{0, 1, 1}, nullCount: 2, itemLen: 1,
+ errContains: "null fixed-size list at index 0 consumed
1 child values"},
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ listType := arrow.FixedSizeListOf(2,
arrow.PrimitiveTypes.Int32)
+ field := arrow.Field{Type: listType, Nullable: true}
+ lr := &listReader{rctx: &readerCtx{mem: mem}, field:
&field}
+
+ builder := array.NewInt32Builder(mem)
+ builder.AppendValues(make([]int32, tc.itemLen), nil)
+ item := builder.NewArray()
+ defer item.Release()
+ builder.Release()
+
+ var validity *memory.Buffer
+ if tc.nullCount > 0 {
+ validity =
memory.NewBufferBytes([]byte{tc.validity})
+ defer validity.Release()
+ }
+
+ out, err := lr.buildFixedSizeListArray(tc.length,
tc.offsets, validity, tc.nullCount, item)
+ if out != nil {
+ out.Release()
+ }
+ require.ErrorContains(t, err, tc.errContains)
+ })
+ }
+}
+
+func TestBuildFixedSizeListArrayConcatenatesSpecializedChildren(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(t, 0)
+
+ build := func(t *testing.T, item arrow.Array) arrow.Array {
+ t.Helper()
+ listType := arrow.FixedSizeListOf(2, item.DataType())
+ field := arrow.Field{Type: listType, Nullable: true}
+ lr := &listReader{rctx: &readerCtx{mem: mem}, field: &field}
+ validity := memory.NewBufferBytes([]byte{0x01})
+ defer validity.Release()
+
+ out, err := lr.buildFixedSizeListArray(2, []int32{0, 2, 2},
validity, 1, item)
+ require.NoError(t, err)
+ t.Cleanup(out.Release)
+
+ list := out.Chunk(0).(*array.FixedSizeList)
+ assert.False(t, list.IsNull(0))
+ assert.True(t, list.IsNull(1))
+ require.Equal(t, 4, list.ListValues().Len())
+ return list.ListValues()
+ }
+
+ t.Run("dictionary", func(t *testing.T) {
+ dictType := &arrow.DictionaryType{IndexType:
arrow.PrimitiveTypes.Int8, ValueType: arrow.BinaryTypes.String}
+ item, err := array.DictArrayFromJSON(mem, dictType, `[0, 1]`,
`["one", "two"]`)
+ require.NoError(t, err)
+ defer item.Release()
+
+ values := build(t, item).(*array.Dictionary)
+ assert.Equal(t, "one", values.ValueStr(0))
+ assert.Equal(t, "two", values.ValueStr(1))
+ assert.True(t, values.IsNull(2))
+ assert.True(t, values.IsNull(3))
+ })
+
+ t.Run("extension", func(t *testing.T) {
+ extType, err := extensions.NewJSONType(arrow.BinaryTypes.String)
+ require.NoError(t, err)
+ builder := array.NewExtensionBuilder(mem, extType)
+ defer builder.Release()
+
builder.StorageBuilder().(*array.StringBuilder).AppendValues([]string{`{"one":
1}`, `{"two": 2}`}, nil)
+ item := builder.NewArray()
+ defer item.Release()
+
+ values := build(t, item).(array.ExtensionArray)
+ assert.True(t, arrow.TypeEqual(extType, values.DataType()))
+ storage := values.Storage().(*array.String)
+ assert.Equal(t, `{"one": 1}`, storage.Value(0))
+ assert.Equal(t, `{"two": 2}`, storage.Value(1))
+ assert.True(t, storage.IsNull(2))
+ assert.True(t, storage.IsNull(3))
+ })
+}
+
func TestChunkedTableRoundTrip(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
defer mem.AssertSize(t, 0)
diff --git a/parquet/pqarrow/encode_arrow_test.go
b/parquet/pqarrow/encode_arrow_test.go
index 6c9e979d..7a4fdd01 100644
--- a/parquet/pqarrow/encode_arrow_test.go
+++ b/parquet/pqarrow/encode_arrow_test.go
@@ -1967,6 +1967,39 @@ func (ps *ParquetIOTestSuite) TestFixedSizeList() {
ps.roundTripTable(mem, tbl, true)
}
+func (ps *ParquetIOTestSuite) TestFixedSizeListNullableValues() {
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(ps.T(), 0)
+
+ bldr := array.NewFixedSizeListBuilder(mem, 3,
arrow.PrimitiveTypes.Int16)
+ defer bldr.Release()
+ vb := bldr.ValueBuilder().(*array.Int16Builder)
+
+ bldr.AppendNull()
+ bldr.Append(true)
+ vb.AppendValues([]int16{1, 2, 3}, nil)
+ bldr.AppendNull()
+
+ data := bldr.NewArray()
+ defer data.Release()
+ field := arrow.Field{Name: "root", Type: data.DataType(), Nullable:
true}
+ tbl := array.NewTableFromSlice(arrow.NewSchema([]arrow.Field{field},
nil), [][]arrow.Array{{data}})
+ defer tbl.Release()
+
+ ps.roundTripTable(mem, tbl, true)
+
+ allNullBuilder := array.NewFixedSizeListBuilder(mem, 3,
arrow.PrimitiveTypes.Int16)
+ defer allNullBuilder.Release()
+ allNullBuilder.AppendNulls(2)
+ allNull := allNullBuilder.NewArray()
+ defer allNull.Release()
+ allNullField := arrow.Field{Name: "root", Type: allNull.DataType(),
Nullable: true}
+ allNullTable :=
array.NewTableFromSlice(arrow.NewSchema([]arrow.Field{allNullField}, nil),
[][]arrow.Array{{allNull}})
+ defer allNullTable.Release()
+
+ ps.roundTripTable(mem, allNullTable, true)
+}
+
// TestFixedSizeListNullableElements verifies that FixedSizeList with nullable
// element type correctly round-trips non-null values through Parquet.
// This is a regression test for https://github.com/apache/arrow-go/issues/584
