viirya opened a new pull request, #59110: URL: https://github.com/apache/spark/pull/59110
### What changes were proposed in this pull request? This is a backport of https://github.com/apache/spark/pull/59087 to branch-4.1. Similar to SPARK-59352, this validates lengths read from the file in the vectorized PLAIN, DELTA_LENGTH_BYTE_ARRAY and DELTA_BYTE_ARRAY readers, mainly for their skip paths. `VectorizedPlainValuesReader`: - Add `readLength()`, which rejects a binary length that is negative or larger than the bytes left in the page, and use it in `readBinary` and `skipBinary`. - Add `VectorizedReaderBase.skipFully(ByteBufferInputStream, long)`, which throws a `ParquetDecodingException` when the page has fewer bytes than requested, and use it in `skipBinary`, `skipBooleans` and all fixed-width skips. `VectorizedDeltaLengthByteArrayReader`: - Validate all value lengths once in `initFromPage`: their count must not exceed the page's value count, each must be non-negative, and their sum must fit in the bytes left in the page. - Check that the rows consumed by `readBinary`, `skipBinary` and `getBytes` have a decoded length. - Rewrite `skipBinary` to sum the lengths of the batch and skip them with a single `skipFully`, replacing the `remaining -= in.skip(remaining)` loop. `VectorizedDeltaByteArrayReader`: - Reject a page with more prefix lengths than values, or whose prefix length count differs from its suffix count. - Get the suffix, which checks that the row was decoded, before reading the prefix length. Differences from the original, from resolving conflicts in the cherry-pick: - branch-4.1 has no Geometry/Geography readers, so the `readGeoData` changes in the three readers and the geo tests (in `VectorizedPlainValuesReaderSuite` and `ParquetDeltaLengthByteArrayEncodingSuite`) are dropped. - branch-4.1 does not have the unsigned long encoding helpers that master has in `VectorizedReaderBase`, so only `skipFully` is added there. ### Why are the changes needed? The skip paths are used when column-index filtering produces row ranges, and they did not validate what they read from the file. `SingleBufferInputStream.skip(n)` returns -1 at the end of the buffer, skips fewer bytes when the buffer is short, and moves the position backwards for a negative `n`. On a corrupt page this can make `VectorizedDeltaLengthByteArrayReader.skipBinary` spin for about 2^31 iterations per value, or make `VectorizedPlainValuesReader.skipBinary` decode the following values from the wrong offset and return wrong results without an error. See https://github.com/apache/spark/pull/59087 for the full list. ### Does this PR introduce _any_ user-facing change? Only for corrupt Parquet files: reading a page with an invalid value length, an inconsistent length count, or too few bytes for the values being skipped now fails with a `ParquetDecodingException` instead of silently returning wrong values or spinning. Valid files are not affected. ### How was this patch tested? New unit tests from the original PR: `VectorizedPlainValuesReaderSuite` (new), and new cases in `ParquetDeltaLengthByteArrayEncodingSuite` and `ParquetDeltaByteArrayEncodingSuite`. On branch-4.1 (Parquet 1.16.0), ran: - `build/sbt sql/scalastyle sql/Test/scalastyle sql/checkstyle sql/Test/checkstyle`: clean for the changed files. `sql/Test/checkstyle` reports one existing error on branch-4.1 in `SpatialReferenceSystemMapperSuite.java` (trailing whitespace), unrelated to this change. - `build/sbt 'sql/testOnly *VectorizedPlainValuesReaderSuite *ParquetDeltaLengthByteArrayEncodingSuite *ParquetDeltaByteArrayEncodingSuite *ParquetEncodingSuite *ParquetVectorizedSuite *ParquetColumnIndexSuite *ParquetIOSuite'`: 160 tests passed. This change also applies cleanly to branch-3.5 (`git cherry-pick` of this commit, no conflicts). On branch-3.5 (Scala 2.12, Parquet 1.13.1, JDK 17), style checks are clean and the same suites pass: 139 tests. So it can be cherry-picked to branch-3.5 when merging, without a separate PR. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code 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]
