viirya commented on code in PR #59117:
URL: https://github.com/apache/spark/pull/59117#discussion_r4176265737


##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedRleValuesReader.java:
##########
@@ -100,12 +100,21 @@ public void initFromPage(int valueCount, 
ByteBufferInputStream in) throws IOExce
       // Initialize for repetition and definition levels
       if (readLength) {
         int length = readIntLittleEndian();

Review Comment:
   Done, with a test.
   



##########
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) {
+        throw new ParquetDecodingException(
+          "Corrupted RLE data: reading past the end of the encoded values");
+      }
+      readGroup();
+    } while (this.currentCount == 0);
+  }
 
+  /**
+   * Reads the next group, which may be empty.
+   */
+  private void readGroup() {
     try {
       int header = readUnsignedVarInt();

Review Comment:
   Thanks. The header length limit has the same issue as the run count cap: a 
6-byte header with zero high bytes (e.g. `94 80 80 80 80 00`) decodes correctly 
today because Java masks the shift amount, so I left it for the follow-up. The 
dictionary id bound needs a per-value check for bit-packed ids, so I'd keep it 
separate too.
   



-- 
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]

Reply via email to