yimingli-vmware opened a new pull request, #3717:
URL: https://github.com/apache/parquet-java/pull/3717
## Rationale for this change
Fixes #3716.
`FixedLenByteArrayPlainValuesReader` hands out `Binary` values that all
share one page-wide `ByteBuffer`, advancing its live position on every
`readBytes()` call. `Binary.ByteBufferBackedBinary.getBytes()` and
`toStringUsingUTF8()` (non-array-backed branch) called
`value.limit(offset + length)` directly on that same shared buffer before
capturing `value.position()`. Since `ByteBuffer.limit()` clamps `position`
down whenever `position > newLimit`, calling `getBytes()` on an earlier
value *after* later values have already advanced the buffer permanently
rewinds the buffer's live position -- corrupting every `readBytes()` call
that follows.
This surfaces as data corruption when reading a repeated (`LIST`)
`FIXED_LEN_BYTE_ARRAY` column with 2+ elements per row across 2+ rows:
record assembly stores each `Binary` and only materializes it once a full
row/group has been built, which is exactly the lazy-after-later-value
pattern that triggers the clamp. Each subsequent row reads back the
previous row's last-written element instead of its own (see #3716 for a
minimal standalone repro).
## What changes are included in this PR?
- `Binary.ByteBufferBackedBinary.getBytes()` and `.toStringUsingUTF8()` now
`duplicate()` the buffer before adjusting position/limit, so the shared
buffer's own position is never mutated.
- Added regression tests to
`TestFixedLenByteArrayPlainValuesWriterReader` that read values out of
the order they were materialized, matching the lazy-consumption pattern
from record assembly, and fail against unpatched 1.18.0.
## Are these changes tested?
Yes -- two new tests
(`testLazyGetBytesDoesNotCorruptSubsequentReadsDirectBuffer`,
`testLazyToStringUsingUTF8DoesNotCorruptSubsequentReadsDirectBuffer`) in
`parquet-column/src/test/java/org/apache/parquet/column/values/plain/TestFixedLenByteArrayPlainValuesWriterReader.java`
fail on the unpatched code and pass with this fix. Also verified against
the full-file-roundtrip repro from #3716.
## Are there any user-facing changes?
No API changes. This fixes a silent data-corruption bug introduced in
1.18.0; no user-facing behavior changes other than correct results.
--
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]
