Skip to content

parquet: return an error instead of panicking when a pushed buffer's length does not match its range - #10564

Merged
alamb merged 1 commit into
apache:mainfrom
ranflarion:parquet-push-buffers-error-on-length-mismatch-upstream
Aug 6, 2026
Merged

parquet: return an error instead of panicking when a pushed buffer's length does not match its range#10564
alamb merged 1 commit into
apache:mainfrom
ranflarion:parquet-push-buffers-error-on-length-mismatch-upstream

Conversation

@ranflarion

@ranflarion ranflarion commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #10563.

Rationale for this change

A short read pushed into the parquet push decoder panics at PushBuffers::push_range ("Range length must match buffer length") although every public entry point that feeds it returns Result. Through the async reader this turns a transient store fault into a reader-thread panic the caller cannot classify or retry; the json and avro readers surface the equivalent fault as a decode error.

What changes are included in this PR?

PushBuffers::push_range and push_ranges return Result<(), ParquetError> instead of asserting (buffer/range length mismatch, and ranges/buffers count mismatch in push_ranges). The error propagates through the crate-internal chain: ParquetMetaDataPushDecoder::push_range/push_ranges (already Result, now use ?), and ParquetDecoderState::push_data -> RemainingRowGroups::push_data -> RowGroupReaderBuilder::push_data, the last two becoming fallible; all their callers were already in Result contexts.

Are these changes tested?

New unit tests in push_buffers.rs cover the accepted case, the short-buffer error, and the count-mismatch error. The existing parquet test suite passes.

Are there any user-facing changes?

Changes to PushBuffers public API; push_range() and push_ranges() now return a Result<(), ParquetError>. Code that previously panicked on mismatched pushes now receives an Err.

@github-actions github-actions Bot added the parquet Changes to the parquet crate label Aug 5, 2026
@Jefffrey Jefffrey added the api-change Changes to the arrow API label Aug 6, 2026
@Jefffrey

Jefffrey commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

marking as api change since PushBuffers is part of public api via push decoder export:

pub use crate::util::push_buffers::PushBuffers;

see: https://docs.rs/parquet/latest/parquet/arrow/push_decoder/struct.PushBuffers.html

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ranflarion and @Jefffrey

@alamb
alamb merged commit 53620e0 into apache:main Aug 6, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change Changes to the arrow API parquet Changes to the parquet crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

parquet: push decoder panics instead of returning an error when a pushed buffer's length does not match its range

3 participants