peter-toth commented on code in PR #59324:
URL: https://github.com/apache/spark/pull/59324#discussion_r4267724037
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedParquetRecordReader.java:
##########
@@ -185,9 +184,8 @@ public VectorizedParquetRecordReader(boolean useOffHeap,
int capacity) {
/**
* Implementation of RecordReader API. Super's delegates to the overload
below, which runs
- * {@link #initializeInternal()}, so this does not run it a second time.
Kept although it only
- * calls super, because removing it fails MiMa with a {@code
DirectAbstractMethodProblem} on
- * {@code org.apache.hadoop.mapreduce.RecordReader.initialize}.
+ * {@link #initializeInternal()}. Kept although it only calls super, since
removing it fails
+ * MiMa.
Review Comment:
Done in
[1410d4684cb](https://github.com/apache/spark/pull/59324/commits/1410d4684cb7dffceb36fb7ecce3c0168e67fda3).
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/LateMaterializationParquetRecordReader.java:
##########
@@ -672,42 +578,31 @@ protected boolean suppliesOwnVector(int slot) {
}
/**
- * Counts a file the filter rejects whole, which happens when every key
column is missing from it
- * and the predicate is constant-false for the value the reader would have
materialized. Every row
- * group the pushed data filter left rows in counts as skipped, with every
projected byte of those
- * rows as avoided. Those are all non-key bytes, since the file has no key
column.
+ * Counts a file whose constant predicate is false. Every row group with
rows the pushed data
+ * filter kept counts as skipped, with all their projected bytes avoided,
since the file has no
+ * key column.
*/
private void recordFileSkipped() {
StorageFilterMetrics m = storageFilter.metrics();
List<BlockMetaData> blocks = fileReader.getRowGroups();
for (int blockIdx = 0; blockIdx < blocks.size(); blockIdx++) {
- // Measured against the rows the pushed data filter kept, which is the
baseline every other
- // skip path uses. Resolving them again is a cache hit whenever the two
can differ, because
- // `getFilteredRecordCount()` at initialize resolved every block's
ranges then.
RowRanges blockRanges = pushedFilterRangesFor(blockIdx,
blocks.get(blockIdx).getRowCount());
long survivingRows = blockRanges.rowCount();
if (survivingRows == 0) continue;
- // The key columns are missing from this file, so they contribute
nothing to the walk, and the
- // whole projection is what a plain read would have transferred.
long avoidedBytes = blockAccounting.compressedBytesForRowRanges(
blockIdx, requestedPaths, blockRanges, survivingRows);
m.recordRowGroupSkipped(survivingRows, avoidedBytes);
}
}
/**
- * The rows of a block the pushed data filter allows, at column-index
granularity, or every row of
- * the block when {@link #pageIndexDisabled} says the page index must not be
used.
- *
- * <p>{@code getRowRanges} asks only whether a filter is pushed, not whether
column-index
- * filtering is still enabled, so this is where that conf is honoured. Every
phase reads within
- * these ranges, so narrowing them by an index the read may not trust would
cost rows.
+ * The rows of a block the pushed data filter allows, at column-index
granularity, or every row
+ * when {@link #pageIndexDisabled}. {@code getRowRanges} does not check that
conf itself.
*/
private RowRanges pushedFilterRangesFor(int blockIdx, long blockRowCount) {
if (blockRowCount == 0) {
- // parquet-mr never writes an empty block, and the plain read path skips
one too. For one,
- // `RowRanges.createSingle(0)` below would build Range(0, -1) and trip
its `from <= to`
- // assertion.
+ // parquet-mr never writes an empty block, but
`RowRanges.createSingle(0)` would fail on
Review Comment:
Done in
[1410d4684cb](https://github.com/apache/spark/pull/59324/commits/1410d4684cb7dffceb36fb7ecce3c0168e67fda3).
--
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]
