You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/08/06 17:17:42 UTC
[PR] fix(parquet/pqarrow): validate reader indexes [arrow-go]
fallintoplace opened a new pull request, #1124:
URL: https://github.com/apache/arrow-go/pull/1124
### What does this change do?\n\nValidate field and row-group indexes before using them in the Arrow Parquet reader.\n\n### Why?\n\nSome public reader paths index the manifest or row-group metadata directly. Invalid input can panic even though the surrounding APIs return errors.\n\n### Tests\n\n- go test ./parquet/pqarrow -run TestFileReaderIndexValidation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet/pqarrow): validate reader indexes [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #1124:
URL: https://github.com/apache/arrow-go/pull/1124
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet/pqarrow): validate reader indexes [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1124:
URL: https://github.com/apache/arrow-go/pull/1124#discussion_r3760311309
##########
parquet/pqarrow/file_reader.go:
##########
@@ -219,6 +219,13 @@ func (fr *FileReader) allRowGroupFactory() itrFactory {
//
// IncludedLeaves and RowGroups are used to specify precisely which leaf indexes and row groups to read a subset of.
func (fr *FileReader) GetFieldReader(ctx context.Context, i int, includedLeaves map[int]bool, rowGroups []int) (*ColumnReader, error) {
+ if i < 0 || i >= len(fr.Manifest.Fields) {
+ return nil, fmt.Errorf("%w: invalid field index chosen %d, there are only %d fields", arrow.ErrIndex, i, len(fr.Manifest.Fields))
Review Comment:
This establishes `errors.Is(err, arrow.ErrIndex)` for `GetFieldReader`, but the identical bounds check in `getColumnReader` still returns a plain `fmt.Errorf`. That leaves `GetColumn` and `RowGroup(...).Column(...).Read` as the adjacent reader-index paths that cannot be classified with `errors.Is`.
Non-blocking for the panic fix, but could we wrap `arrow.ErrIndex` there too and add one assertion so the typed index-error contract is consistent?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org