HyukjinKwon commented on code in PR #59117:
URL: https://github.com/apache/spark/pull/59117#discussion_r4151893410
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedRleValuesReader.java:
##########
@@ -976,14 +984,26 @@ private int readIntLittleEndianPaddedOnBitWidth() throws
IOException {
}
/**
- * Reads the next group. Returns false if no more group available.
+ * Reads the next non-empty group, so that `currentCount` is positive on
return.
+ * <p>
+ * Callers never read more values than the page declares, so running out of
encoded data here
+ * means the page is corrupted. With a bit width of 0, `initFromPage`
already provides all the
+ * values of the page, so reaching here also means reading past the end.
*/
- private boolean readNextGroup() {
- if (in.available() <= 0) {
- currentCount = 0;
- return false;
- }
+ private void readNextGroup() {
+ do {
+ if (bitWidth == 0 || in.available() <= 0) {
Review Comment:
Thanks for checking, you're right. In parquet-common 1.18.1 both
`SingleBufferInputStream.read()` and `MultiBufferInputStream.read()` throw
`EOFException` at the end of the stream instead of returning -1, so a truncated
header fails with "Corrupted RLE data: failed to read from input stream" rather
than hanging. I withdraw this one; a test like the `[4, 0x80]` case you
suggested on L1048 would pin it.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4131725984","thread_id":"inline:4131725984","verdict_sha256":"31584873b9168742bc366668db424b5e05487283210d6f3f5e3b76917b7cf8d2"}
-->
--
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]
