viirya opened a new pull request, #59118: URL: https://github.com/apache/spark/pull/59118
### What changes were proposed in this pull request? Follow-up of SPARK-59812 (#59087), which made the vectorized PLAIN, DELTA_LENGTH_BYTE_ARRAY and DELTA_BYTE_ARRAY readers fail with a `ParquetDecodingException` on corrupt pages. This does the same for `VectorizedByteStreamSplitValuesReader` (added by SPARK-56894), which did not check that reads and skips stay within the page: - `initFromPage` rejects a page whose length is not a multiple of the value width, and a page with more encoded values than the page value count (which includes nulls, so it is an upper bound). parquet-java 1.18.1's `ByteStreamSplitValuesReader` rejects both. - Batch reads (`readIntegers`, `readLongs`, `readFloats`, `readDoubles`, their widening/narrowing variants, `readBytes`, `readShorts`, `readBinary`, `readFixedLenByteArray`) and all skips check once per batch that the page has enough values left. The per-value loops are unchanged. - Single-value reads (`readInteger`, `readLong`, `readFloat`, `readDouble`, `readByte`, `readShort`, `readBinary(len)`) check that a value is left. ### Why are the changes needed? On a corrupt page: - The skip methods only did `offset += total`, so skipping past the end of the page was never detected. - Reading past the end (or reading after such a skip) failed with an `ArrayIndexOutOfBoundsException` instead of a `ParquetDecodingException`. - A page whose length is not a multiple of the value width was accepted. BYTE_STREAM_SPLIT stores no value count, and the Parquet spec allows no padding in a data page, so the stream boundaries cannot be derived for such a page: for example, a page that lost a byte has every stream start at the wrong offset, so every value is decoded from the wrong bytes. In addition, reads past the last whole value silently returned values assembled from the trailing bytes instead of failing. ### Does this PR introduce _any_ user-facing change? Only for corrupt Parquet files: reading such a BYTE_STREAM_SPLIT page with the vectorized reader now fails with a `ParquetDecodingException` instead of an `ArrayIndexOutOfBoundsException` or silently wrong values. Valid files are not affected. The vectorized BYTE_STREAM_SPLIT reader is not in a released version yet. ### How was this patch tested? New tests in `ParquetByteStreamSplitEncodingSuite`, run for INT32, INT64, FLOAT and DOUBLE widths, plus an FLBA test with an odd width (5): - Negative: every applicable batch read and skip past the end of the page, after a skip, and with a negative count; every single-value read past the end; page lengths with 1 to W-1 extra or missing bytes; more encoded values than the page value count; reads and skips on an empty page. - Positive: reads and skips that end exactly at the end of the page, followed by empty reads and skips; reading the last value after a skip; fewer encoded values than the page value count (nullable columns). All the new negative tests fail without this change. Existing suites also pass: `ParquetEncodingSuite` (including the BYTE_STREAM_SPLIT round trip with nullable columns), `ParquetVectorizedSuite`, `ParquetColumnIndexSuite` and `ParquetIOSuite`. The single-value check adds one compare per value. A per-value A/B microbenchmark (`readInteger`, `readLong`, `readDouble`, `readBinary(12)`, 1M values, JDK 21) showed no measurable difference. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) This pull request and its description were written by Isaac. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
