iemejia commented on code in PR #55924:
URL: https://github.com/apache/spark/pull/55924#discussion_r3578171766
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedDeltaByteArrayReader.java:
##########
@@ -79,20 +83,25 @@ private void readValues(int total, WritableColumnVector c,
int rowId) {
// value of the page should have an empty prefix, it may not
// because of PARQUET-246.
int prefixLength = prefixLengthVector.getInt(currentRow);
- ByteBuffer suffix = suffixReader.getBytes(currentRow);
- byte[] suffixArray = suffix.array();
- int suffixLength = suffix.limit() - suffix.position();
+ int suffixLength = suffixReader.getSuffixLength(currentRow);
int length = prefixLength + suffixLength;
- // We have to do this to materialize the output
+ // Grow prevBuf if needed, preserving the prefix bytes already in place.
Review Comment:
Good catch -- added coverage in `ParquetDeltaByteArrayEncodingSuite`:
- `longPrefixValues` holds values up to 271 bytes that share long (up to
270-byte) prefixes, so the `length > prevBuf.length` grow branch fires with
`prefixLength > 0`, exercising the `System.arraycopy(prevBuf, 0, newBuf, 0,
prefixLength)` bounds you flagged.
- `"buffer grows preserving long shared prefixes"` runs them through
`readBinary`, and `"buffer grows preserving long shared prefixes with skip"`
alternates `readBinary`/`skipBinary` so a skipped value triggers the grow and
the following read verifies `skipBinary` kept `prevBuf` intact.
- `"setPreviousReader recovers a long previous value across pages
(PARQUET-246)"` ports parquet-java's corrupt-page reassembly test with a
>64-byte previous value, covering the grow + deep-copy in `setPreviousReader`.
All three fail (wrong bytes / AIOOBE) if the grow path drops the prefix, and
pass on the current implementation.
--
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]
