LuciferYang opened a new pull request, #9571:
URL: https://github.com/apache/paimon/pull/9571
### Purpose
close #9570
`VectorizedDeltaLengthByteArrayReader.skipBinary` walked each value's
declared length like this:
```java
int remaining = lengthsVector.getInt(currentRow + i);
while (remaining > 0) {
remaining -= in.skip(remaining);
}
```
On a truncated or corrupt page the declared length exceeds the bytes left,
and `ByteBufferInputStream.skip` returns -1 once the stream is dry, so
`remaining` grows by one per iteration. The loop ends only when the int wraps
negative, about 2^31 iterations, measured at 0.9s per value, and then the
method returns with no error at all. The page stream is empty by then, so a
later read on the same page reports `Failed to read N bytes` against a value
that is not the corrupt one, and if the skip was the page's last operation
nothing is reported at all. Reading the same page instead of skipping it fails
cleanly and immediately, since `readBinary` and `getBytes` call
`in.slice(length)` and convert the `EOFException`; the skip path was the only
one that did not.
The fix uses `in.skipFully(length)`, which skips or throws `EOFException`,
matching those two methods and parquet-mr's own
`DeltaLengthByteArrayValuesReader.skip`. That alone would regress on a negative
declared length: `SingleBufferInputStream.skip` computes `Math.min(remaining,
n)` and would move the position backwards, where the old loop did nothing while
`currentRow` advanced, misaligning the reader silently. So a negative length is
rejected explicitly. The exception carries its cause because the `EOFException`
message says how far the skip got, which the wrapper text drops.
### Tests
Both go in the existing `DeltaLengthByteArrayEncodingTest`, which already
covers this reader including two skip cases. One writes the page normally and
then drops three bytes off the data section, so the lengths still describe more
data than the page holds. The other writes a length section that decodes to -1
through `DeltaBinaryPackingValuesWriterForInteger`, since no writer produces
that.
Against the unfixed reader both fail with "Expected ParquetDecodingException
to be thrown, but nothing was thrown", the truncation one after spinning for
0.9s. That is also the evidence for the wraparound described above.
`mvn -pl paimon-format test` on JDK 8: 598 tests, 0 failures.
`spotless:check` and `checkstyle:check` are clean.
Not included, to keep this to one thing:
`VectorizedPlainValuesReader.skipBinary` is a bare `in.skip(len)` that ignores
the return value too. Its symptom is different, a silent under-skip rather than
a spin, and PLAIN is the common encoding, so making it throw would turn files
that read today, misaligned, into hard failures. That seems worth its own
discussion.
--
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]
