Gabriel39 commented on code in PR #65602:
URL: https://github.com/apache/doris/pull/65602#discussion_r3580038000
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -1314,8 +1333,17 @@ bool
ParquetScanScheduler::prepare_current_row_group_reader(
if (file_context.metadata == nullptr) {
return false;
}
- const auto ranges = build_row_group_prefetch_ranges(
- *file_context.metadata, file_schema,
request_scan_columns(request), row_group_idx);
+ // MergeRangeFileReader can merge forward from the range containing the
current ReadAt() into
+ // later registered ranges. Registering lazy chunks before predicate
evaluation would therefore
+ // let a predicate read fetch output bytes that a fully filtered row group
never materializes.
+ // Keep filtered scans predicate-only here. Reads for a surviving batch's
lazy columns fall
+ // outside these ranges and use the underlying reader directly; unfiltered
scans can still
+ // coalesce all projected chunks because every output column is guaranteed
to be consumed.
+ const auto merge_range_columns = request.conjuncts.empty() &&
request.delete_conjuncts.empty()
+ ? request_scan_columns(request)
Review Comment:
Thanks for pointing out the `pred0 / lazy / pred1` hollow-read case. After
comparing this path with v1, we decided not to introduce a stronger
physical-I/O guarantee in this PR.
V1 builds `MergeRangeFileReader` ranges from all projected file columns in
physical order and intentionally permits bounded read amplification to reduce
remote request count. Its lazy-read contract avoids reading, decoding, and
materializing the lazy column reader when no row survives; it does not
guarantee that merge-range never fetches bytes belonging to a lazy chunk. The
pending-skip change in this PR has the same scope: it avoids repeated Arrow
`SkipRecords` calls and drops lazy readers for a fully filtered row group.
Restricting v2 to predicate-only ranges made merge-range selection diverge
from v1 and still could not prevent hollow reads between predicate chunks.
Fully disabling hollow merging would be a separate performance tradeoff that
needs object-storage benchmarks. We therefore restored the v1-aligned
all-projected-column range policy and removed the stronger `MergedBytes`
assertion. The condition-cache key test improvement from the previous follow-up
remains.
--
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]
