peter-toth commented on code in PR #59324:
URL: https://github.com/apache/spark/pull/59324#discussion_r4267722238
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/LateMaterializationParquetRecordReader.java:
##########
@@ -190,11 +151,10 @@ public class LateMaterializationParquetRecordReader
extends VectorizedParquetRec
/**
* Whether {@code parquet.filter.columnindex.enabled} is false, so this
file's page index must not
- * be used. That turns off phase 0 and every partial read. A wrong index
would pair a row's key
- * with another row's values, or return rows the filter rejects once
splicing is given up. The
- * post-scan Filter catches neither, since it takes the reader's mark on
those rows. Parquet's
- * partial read does not check the conf itself. Read off {@link
#readOptions}, so it agrees with
- * the row count {@link #fileReader} reported.
+ * be used. That turns off phase 0 and every partial read, which parquet
does not check itself.
+ * It is the way out of a wrong page index, which would pair a row's key
with another row's
+ * values, and which the post-scan Filter cannot catch on the rows this
reader marks checked.
+ * Read off {@link #readOptions}, so it agrees with {@link #fileReader}.
Review Comment:
Done in
[1410d4684cb](https://github.com/apache/spark/pull/59324/commits/1410d4684cb7dffceb36fb7ecce3c0168e67fda3):
`so it agrees with the row count {@link #fileReader} reported.` I left the
splicing case out. The comment gives one example of what a wrong index does,
and the conf doc covers wrong rows in general.
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetBlockAccounting.java:
##########
@@ -38,14 +38,8 @@
/**
* What a partial read of a Parquet file transfers, and whether one is
possible at all, answered
- * from the footer the {@link ParquetFileReader} already holds. Every method
here is metadata
- * arithmetic and causes no IO of its own, which is what lets a reader report
these numbers per row
- * group. For a strict subset of a block that holds only once the block's
column index store is
- * built, see {@link #compressedBytesForRowRanges}. A walk before that would
read the offset indexes
- * itself and cache a store over the reader's current columns.
- *
- * <p>One instance per reader, which reads one split of a file, because the
warning below is
- * reported once per instance.
+ * from the metadata the {@link ParquetFileReader} already holds, with no IO
of its own. One
Review Comment:
Done in
[0c77f7ddd22](https://github.com/apache/spark/pull/59324/commits/0c77f7ddd22e4f70db4072d4db39d832ed53dbe7):
`with no IO of its own once the block's store is built, see {@link
#compressedBytesForRowRanges}`.
--
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]
