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 c12dd887 fix(parquet/metadata): validate row-group ordinals in
PageIndexReader.WillNeed (#1057)
c12dd887 is described below
commit c12dd8872dd357132305841a5592f4fe2783c745
Author: Minh Vu <[email protected]>
AuthorDate: Wed Aug 5 18:54:11 2026 +0200
fix(parquet/metadata): validate row-group ordinals in
PageIndexReader.WillNeed (#1057)
### Rationale for this change
`PageIndexReader.WillNeed` accessed row-group metadata before validating
requested ordinals. Negative and oversized values therefore caused a
runtime bounds panic even though the method returns an error.
### What changes are included in this PR?
Validate every ordinal first and return an `arrow.ErrIndex`-wrapped
error for invalid values.
### Are these changes tested?
Yes. The regression test covers both sides of the valid range.
`go test ./parquet/metadata -run
TestPageIndexReaderWillNeedRejectsInvalidRowGroups`
### Are there any user-facing changes?
Invalid row-group hints now return an index error instead of panicking.
---
parquet/metadata/page_index.go | 16 +++++++++++++---
parquet/metadata/page_index_internal_test.go | 20 ++++++++++++++++++++
2 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/parquet/metadata/page_index.go b/parquet/metadata/page_index.go
index f3c9ef3a..a335f886 100644
--- a/parquet/metadata/page_index.go
+++ b/parquet/metadata/page_index.go
@@ -557,10 +557,13 @@ func (r *PageIndexReader) RowGroup(i int)
(*RowGroupPageIndexReader, error) {
}
func (r *PageIndexReader) WillNeed(rgIndices, colIndices []int32, selection
PageIndexSelection) error {
- if r.idxReadRanges == nil {
- r.idxReadRanges = make(map[int32]rgIndexReadRange)
+ for _, ordinal := range rgIndices {
+ if ordinal < 0 || int(ordinal) >= r.FileMetadata.NumRowGroups()
{
+ return fmt.Errorf("%w: row group ordinal %d out of
range", arrow.ErrIndex, ordinal)
+ }
}
+ ranges := make(map[int32]rgIndexReadRange, len(rgIndices))
for _, ordinal := range rgIndices {
readRange, err :=
determinePageIndexRangesInRowGroup(r.FileMetadata.RowGroup(int(ordinal)),
colIndices)
if err != nil {
@@ -576,7 +579,14 @@ func (r *PageIndexReader) WillNeed(rgIndices, colIndices
[]int32, selection Page
// mark offset index as not requested
readRange.OffsetIndex = nil
}
- r.idxReadRanges[int32(ordinal)] = readRange
+ ranges[int32(ordinal)] = readRange
+ }
+
+ if r.idxReadRanges == nil {
+ r.idxReadRanges = make(map[int32]rgIndexReadRange)
+ }
+ for ordinal, readRange := range ranges {
+ r.idxReadRanges[ordinal] = readRange
}
// TODO: possibly use read ranges to prefetch data of the input
return nil
diff --git a/parquet/metadata/page_index_internal_test.go
b/parquet/metadata/page_index_internal_test.go
index 49a74176..594fe111 100644
--- a/parquet/metadata/page_index_internal_test.go
+++ b/parquet/metadata/page_index_internal_test.go
@@ -21,6 +21,7 @@ import (
"strconv"
"testing"
+ "github.com/apache/arrow-go/v18/arrow"
"github.com/apache/arrow-go/v18/parquet"
format "github.com/apache/arrow-go/v18/parquet/internal/gen-go/parquet"
"github.com/apache/arrow-go/v18/parquet/internal/thrift"
@@ -72,6 +73,25 @@ func constructFakeMetadata(rowGroupRanges []PageIndexRanges)
*FileMetaData {
return meta
}
+func TestPageIndexReaderWillNeedRejectsInvalidRowGroups(t *testing.T) {
+ meta := constructFakeMetadata([]PageIndexRanges{{-1, -1, -1, -1}})
+ reader := &PageIndexReader{FileMetadata: meta}
+
+ for _, ordinal := range []int32{-1, 1} {
+ err := reader.WillNeed([]int32{ordinal}, nil,
PageIndexSelection{})
+ require.ErrorIs(t, err, arrow.ErrIndex)
+ }
+}
+
+func TestPageIndexReaderWillNeedDoesNotPartiallyMutateOnError(t *testing.T) {
+ meta := constructFakeMetadata([]PageIndexRanges{{-1, -1, -1, -1}})
+ reader := &PageIndexReader{FileMetadata: meta}
+
+ err := reader.WillNeed([]int32{0, 1}, nil, PageIndexSelection{})
+ require.ErrorIs(t, err, arrow.ErrIndex)
+ require.Empty(t, reader.idxReadRanges)
+}
+
// validates that determinePagteIndexRangesInRowGroup selects the expected
// file offsets and sizes or returns false when the row group doesn't have
// a page index
