Repository navigation
Conversation
This is a follow-up of SPARK-59812. It makes `VectorizedRleValuesReader`, which decodes definition/repetition levels, dictionary ids and RLE booleans, fail with a `ParquetDecodingException` when the encoded data ends early or a header is invalid.
- `readNextGroup()` throws when there is no encoded data left, instead of returning false. Callers never read more values than the page declares, so running out of data always means a corrupt page. All the `if (currentCount == 0 && !readNextGroup()) break;` loop guards become `if (currentCount == 0) readNextGroup();`. With a bit width of 0, `initFromPage` already provides every value of the page, so reaching `readNextGroup()` also means reading past the end.
- `readNextGroup()` skips runs of length 0, so `currentCount` is always positive when it returns. The existing group decoding moves unchanged into a `readGroup()` helper that `readNextGroup()` calls in a loop. The Parquet spec does not allow runs of length 0; skipping them keeps what the batch paths already did, so no file that could be read before fails because of them.
- For a bit-packed run, check that `numGroups * 8` does not overflow and that `numGroups * bitWidth` bytes are left before allocating the value buffer.
- `initFromPage` rejects a level length that is cut off, negative or larger than the bytes left in the page, and a dictionary id bit width above 32.
- `VectorizedColumnReader` now also adds the page or dictionary and the column to a `ParquetDecodingException` thrown while initializing a page (in `readPageV1` and `readPageV2`) or decoding the dictionary page, as it already did for an `IOException`, so a corrupt length still reports the column. It wraps it in a new `ParquetDecodingException` that includes the original message, so the exception type stays the same.
- A read error inside a run header or an RLE value (e.g. the data ends in the middle of a header) now also starts with "Corrupted RLE data".
- A dictionary-encoded page with no data at all (not even the bit width byte) no longer decodes every value as id 0. Reading from it fails, like parquet-java's `DictionaryValuesReader` ("Attempt to read from empty page"). An all-null page, which reads no ids, still works.
The added checks run once per run or page, not per value.
SPARK-34863 made `readNextGroup()` return false at the end of the stream. Before that, it called `in.read()`, which throws an `EOFException` that was wrapped in a `ParquetDecodingException`. Since then, on a corrupt page:
- The batch read and skip loops stop early, so dictionary ids are only partially read or skipped, and the query returns wrong results without an error.
- For definition and repetition levels, `readBatch` and `readBatchRepeated` return without making progress, and `VectorizedColumnReader.readBatch` loops forever because `rowsToReadInBatch` and `valuesToReadInPage` never change.
- For repeated columns, when only the definition levels end early, the missing definition levels are silently left as 0, which produces wrong nested values.
- `readBooleans` ignores the return value, and loops forever at the end of the stream.
- `readInteger` ignores the return value and decrements `currentCount` below 0, returning the last run's value again. It also returns the value of a run of length 0 instead of skipping it.
- A bit-packed header with a large group count overflows `numGroups * 8` and `numGroups * bitWidth` (the latter was added by SPARK-56895). `in.slice` with a negative length moves a `SingleBufferInputStream` backwards, and a large count allocates up to an `int[2^31 - 8]` buffer before noticing that the bytes are missing.
- In Parquet 1.18.1, `SingleBufferInputStream.sliceBuffers` only rejects `length > remaining`, so a negative level length moves the stream backwards and the next section of the page is decoded from the wrong offset.
When the encoded data ends early, the non-vectorized reader already fails: parquet-java's `RunLengthBitPackingHybridDecoder` throws "Reading past RLE/BitPacking stream.".
This PR does not change how a last bit-packed run that is shorter than its padding is handled. Some non-compliant writers produce it (e.g. Polars before pola-rs/polars#13883). The vectorized reader already rejected it before this change, because `in.slice` threw an `EOFException`, while parquet-java and Arrow C++ (apache/arrow#47992) accept it when the bytes cover the values that are read. Accepting it is left for a follow-up, SPARK-59853.
A bit-packed run whose bytes are all present is still not bounded by the number of values in the page, so a corrupt header can still make the reader allocate a buffer much larger than the page needs. Rejecting such runs would also fail non-compliant pages that decode correctly today (e.g. 9 values written as a run of 3 groups), so bounding them is left for a follow-up.
Only for corrupt or non-compliant Parquet files: reading a page whose levels, dictionary ids or RLE booleans end early, or have an invalid length, bit width or bit-packed header, now fails with a `ParquetDecodingException` instead of returning wrong values, spinning forever, or allocating a buffer for a bit-packed run whose bytes are missing. Valid files are not affected. With `spark.sql.files.ignoreCorruptFiles=true`, the rest of such a file is now skipped.
A few of these pages could be read before and give the right result by chance, and now fail:
- Dictionary ids (or RLE boolean values) that end early, when every row after the cut is skipped by column index filtering, e.g. the rest of the page before a row range in a later page. The skip stopped silently at the end of the data. This was possible since 3.3.0 (SPARK-34863); 3.2 failed the same way as this PR.
- A dictionary-encoded page whose data is empty (no bit width byte, which the spec requires), when all its non-null rows are skipped, or when the dictionary has a single entry, so that decoding every id as 0 happened to be right. This was the behavior since the vectorized reader was added in 2.0.0.
Selecting rows after the cut, or a dictionary with more than one entry, returns wrong values for the same pages without this PR. With `spark.sql.files.ignoreCorruptFiles=true`, such a file is no longer read in full: from the batch that reaches such a page, the rest of the file, including later row groups, is skipped with only a warning in the log.
New unit tests in `VectorizedRleValuesReaderSuite`. The ones on hand-built dictionary id, boolean and length sections (9 of 14) run both on a single buffer and on the same data split into 1-byte buffers, so that every part of the data longer than 1 byte, including a slice of it, is read from a `MultiBufferInputStream`. Negative tests check the expected message, and the ones that read values run on a separate thread with a timeout, since some looped forever before this change.
- Truncated dictionary ids on `readIntegers`, `skipIntegers` and `readInteger`; truncated booleans on `readBooleans`; truncated definition levels on `readBatch` (with and without definition level output, and with row ranges that skip past the truncation); truncated repetition levels on `readBatchRepeated`; truncated definition levels on `readBatchRepeated` (with and without row ranges); reading past a page with bit width 0.
- Cut-off, negative and oversized (up to `Int.MaxValue`) level lengths, and dictionary id bit widths above 32.
- Bit-packed runs that are truncated, that overflow `currentCount` to 0 or a negative value, or that would allocate a huge buffer.
- Data ending inside a run header or an RLE value.
- A dictionary-encoded page with no data, and runs of length 0.
- Positive cases: reads and skips that end exactly at the end of the encoded values for mixed RLE and bit-packed dictionary ids and booleans, with a level length that covers exactly the rest of the page.
New end-to-end tests in `ParquetIOSuite` corrupt a written file: a shortened definition level run fails instead of hanging, and an oversized length of the definition levels (V1 page) or of RLE boolean values (V2 page) reports the page and column once. A new test in `ParquetVectorizedSuite` checks that a corrupted dictionary page reports the column.
All the new negative tests fail without this change. Existing suites also pass: `ParquetEncodingSuite`, `ParquetVectorizedSuite`, `ParquetColumnIndexSuite`, `ParquetIOSuite`, `ParquetV1QuerySuite`, `ParquetV2QuerySuite`, `ParquetDeltaByteArrayEncodingSuite`, `ParquetDeltaLengthByteArrayEncodingSuite`, `VectorizedPlainValuesReaderSuite` and `ParquetVectorUpdaterSuite`.
Generated-by: Claude Code (Claude Opus 5.5)
Closes apache#59117 from viirya/SPARK-59832-rle-truncated.
Authored-by: Liang-Chi Hsieh <[email protected]>
Signed-off-by: Liang-Chi Hsieh <[email protected]>
(cherry picked from commit 605deea)
HyukjinKwon
approved these changes
Oct 15, 2026
dongjoon-hyun
approved these changes
Oct 15, 2026
dongjoon-hyun
left a comment
Member
There was a problem hiding this comment.
+1, LGTM (Pending CIs).
dongjoon-hyun
pushed a commit
that referenced
this pull request
Oct 15, 2026
… reader ### What changes were proposed in this pull request? Backport of #59117 to branch-4.x. The cherry-pick conflicted only on code that SPARK-59620 added to master, which is not on this branch: an import in `VectorizedColumnReader` and `VectorizedRleValuesReaderSuite`, and a doc comment next to the new `readBatchRepeated` bridge in `ParquetTestAccess`. They are left out; otherwise the change is the same as on master. This commit also applies cleanly to branch-4.3, so it can be cherry-picked there when merging. The same change was built and tested on branch-4.3. ### Why are the changes needed? Same as #59117. On a corrupt page, the vectorized RLE reader, which decodes definition/repetition levels, dictionary ids and RLE booleans, silently returned wrong values or looped forever when the encoded data ended early, and a corrupt bit-packed header could overflow `currentCount` or allocate a huge buffer. The reader now fails with a `ParquetDecodingException`. ### Does this PR introduce _any_ user-facing change? Only for corrupt or non-compliant Parquet files, as described in #59117: such pages now fail with a `ParquetDecodingException` instead of returning wrong values or hanging, and with `spark.sql.files.ignoreCorruptFiles=true` the rest of such a file is skipped. A few of these pages could be read before and gave the right result by chance; #59117 lists them. ### How was this patch tested? The 18 tests added in #59117 pass on this branch, as do the existing tests in `ParquetEncodingSuite`, `ParquetVectorizedSuite`, `ParquetColumnIndexSuite`, `ParquetIOSuite`, `ParquetV1QuerySuite`, `ParquetV2QuerySuite`, `ParquetDeltaByteArrayEncodingSuite`, `ParquetDeltaLengthByteArrayEncodingSuite`, `VectorizedPlainValuesReaderSuite` and `ParquetVectorUpdaterSuite`. ``` build/sbt 'sql/testOnly *VectorizedRleValuesReaderSuite *ParquetEncodingSuite *ParquetVectorizedSuite *ParquetIOSuite' ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) Closes #59481 from viirya/SPARK-59832-branch-4.x. Authored-by: Liang-Chi Hsieh <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun
pushed a commit
that referenced
this pull request
Oct 15, 2026
… reader ### What changes were proposed in this pull request? Backport of #59117 to branch-4.x. The cherry-pick conflicted only on code that SPARK-59620 added to master, which is not on this branch: an import in `VectorizedColumnReader` and `VectorizedRleValuesReaderSuite`, and a doc comment next to the new `readBatchRepeated` bridge in `ParquetTestAccess`. They are left out; otherwise the change is the same as on master. This commit also applies cleanly to branch-4.3, so it can be cherry-picked there when merging. The same change was built and tested on branch-4.3. ### Why are the changes needed? Same as #59117. On a corrupt page, the vectorized RLE reader, which decodes definition/repetition levels, dictionary ids and RLE booleans, silently returned wrong values or looped forever when the encoded data ended early, and a corrupt bit-packed header could overflow `currentCount` or allocate a huge buffer. The reader now fails with a `ParquetDecodingException`. ### Does this PR introduce _any_ user-facing change? Only for corrupt or non-compliant Parquet files, as described in #59117: such pages now fail with a `ParquetDecodingException` instead of returning wrong values or hanging, and with `spark.sql.files.ignoreCorruptFiles=true` the rest of such a file is skipped. A few of these pages could be read before and gave the right result by chance; #59117 lists them. ### How was this patch tested? The 18 tests added in #59117 pass on this branch, as do the existing tests in `ParquetEncodingSuite`, `ParquetVectorizedSuite`, `ParquetColumnIndexSuite`, `ParquetIOSuite`, `ParquetV1QuerySuite`, `ParquetV2QuerySuite`, `ParquetDeltaByteArrayEncodingSuite`, `ParquetDeltaLengthByteArrayEncodingSuite`, `VectorizedPlainValuesReaderSuite` and `ParquetVectorUpdaterSuite`. ``` build/sbt 'sql/testOnly *VectorizedRleValuesReaderSuite *ParquetEncodingSuite *ParquetVectorizedSuite *ParquetIOSuite' ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) Closes #59481 from viirya/SPARK-59832-branch-4.x. Authored-by: Liang-Chi Hsieh <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 45f1cc8) Signed-off-by: Dongjoon Hyun <[email protected]>
Member
Member
Author
|
Thank you @HyukjinKwon @dongjoon-hyun ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Backport of #59117 to branch-4.x.
The cherry-pick conflicted only on code that SPARK-59620 added to master, which is not on this branch: an import in
VectorizedColumnReaderandVectorizedRleValuesReaderSuite, and a doc comment next to the newreadBatchRepeatedbridge inParquetTestAccess. They are left out; otherwise the change is the same as on master.This commit also applies cleanly to branch-4.3, so it can be cherry-picked there when merging. The same change was built and tested on branch-4.3.
Why are the changes needed?
Same as #59117. On a corrupt page, the vectorized RLE reader, which decodes definition/repetition levels, dictionary ids and RLE booleans, silently returned wrong values or looped forever when the encoded data ended early, and a corrupt bit-packed header could overflow
currentCountor allocate a huge buffer. The reader now fails with aParquetDecodingException.Does this PR introduce any user-facing change?
Only for corrupt or non-compliant Parquet files, as described in #59117: such pages now fail with a
ParquetDecodingExceptioninstead of returning wrong values or hanging, and withspark.sql.files.ignoreCorruptFiles=truethe rest of such a file is skipped. A few of these pages could be read before and gave the right result by chance; #59117 lists them.How was this patch tested?
The 18 tests added in #59117 pass on this branch, as do the existing tests in
ParquetEncodingSuite,ParquetVectorizedSuite,ParquetColumnIndexSuite,ParquetIOSuite,ParquetV1QuerySuite,ParquetV2QuerySuite,ParquetDeltaByteArrayEncodingSuite,ParquetDeltaLengthByteArrayEncodingSuite,VectorizedPlainValuesReaderSuiteandParquetVectorUpdaterSuite.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)