MaxGekk commented on code in PR #55924:
URL: https://github.com/apache/spark/pull/55924#discussion_r3576551395
##########
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:
This grow branch (and its twin in `skipBinary`) is never exercised by the
unit suite: `ParquetDeltaByteArrayEncodingSuite` uses
`getRandomStringSamples(_, 32)` — values capped at 32 bytes — but `prevBuf`
starts at 64, so `length > prevBuf.length` never fires there. It's also the
branch most likely to regress silently (the `System.arraycopy(prevBuf, 0,
newBuf, 0, prefixLength)` bounds depend on the `prefixLength <= prevBuf.length`
invariant).
Worth adding a case with values > 64 bytes that share long prefixes, run
through both `readBinary` and `skipBinary` (and ideally the cross-page
`setPreviousReader` copy). I confirmed the path is correct today via a local
test, so this is purely closing the coverage gap. The `len=64` benchmark
exercises it but isn't a correctness gate.
--
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]
