github-actions[bot] commented on code in PR #67906:
URL: https://github.com/apache/doris/pull/67906#discussion_r3998826306
##########
be/src/exec/rowid_fetcher.cpp:
##########
@@ -478,14 +479,28 @@ Status
RowIdStorageReader::read_external_row_from_file_mapping(
std::unique_ptr<RuntimeProfile> sub_runtime_profile =
std::make_unique<RuntimeProfile>("ExternalRowIDFetcher");
{
- std::unique_ptr<FileScanner> vfile_scanner_ptr =
- FileScanner::create_unique(runtime_state.get(),
sub_runtime_profile.get(),
- &rpc_scan_params,
&colname_to_slot_id, &tuple_desc);
-
-
RETURN_IF_ERROR(vfile_scanner_ptr->prepare_for_read_lines(scan_range_desc));
- RETURN_IF_ERROR(vfile_scanner_ptr->read_lines_from_range(
- scan_range_desc, read_ids, &scan_blocks[idx], external_info,
- &fetch_statistics[idx].init_reader_ms,
&fetch_statistics[idx].get_block_ms));
+ const auto format_type = scan_range_desc.__isset.format_type ?
scan_range_desc.format_type
+ :
rpc_scan_params.format_type;
+ if ((format_type == TFileFormatType::FORMAT_PARQUET ||
+ format_type == TFileFormatType::FORMAT_ORC) &&
+ FileScannerV2::is_supported(rpc_scan_params, scan_range_desc)) {
Review Comment:
[P1] Keep phase two on the scanner selected for phase one
`FileScanLocalState::_should_use_file_scanner_v2()` requires
`enable_file_scanner_v2` to be present and true, but this condition checks only
format/support. An eligible Top-N lazy Hive/Iceberg/file-TVF Parquet or ORC
query with the flag explicitly false therefore scans phase one with V1, while
its `FileMapping` records no scanner flavor and phase two silently enters
standalone V2. This defeats the rollout/rollback control (and older
absent-option payloads have the same BE mismatch). Please reuse the phase-one
selection policy here or persist the actual scanner choice in `FileMapping`,
and cover false/unset end to end.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -556,7 +556,28 @@ Status build_native_row_group_read_plans(
row_group_plan.row_group_id = row_group_idx;
row_group_plan.first_file_row = row_group_first_rows[row_group_idx];
row_group_plan.row_group_rows = row_group.num_rows;
- row_group_plan.selected_ranges = {{.start = 0, .length =
row_group.num_rows}};
+ if (request.row_ids.has_value()) {
Review Comment:
[P1] Do not prefetch whole chunks for sparse row-ID reads
This branch narrows logical `selected_ranges`, but `read_by_rows()` supplies
no conjuncts, so row-group open treats the request as unfiltered and eagerly
prefetches every projected non-predicate Column Chunk in full. With an admitted
cached remote reader, fetching one Top-N row can therefore download and cache
every missing block in large projected chunks; the dry-run prefetch also clears
reader/cache statistics, so the amplification is absent from the phase-two
profile. Treat a present `row_ids` request as selective for prefetch (or
restrict prefetch to validated selected-page ranges), preserve accounting, and
add cached-remote sparse-row coverage.
--
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]
