Gabriel39 commented on code in PR #67906:
URL: https://github.com/apache/doris/pull/67906#discussion_r3998999821


##########
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:
   Fixed in 6a64a67fee. The second-phase dispatch now reuses 
FileScanLocalState's phase-one selection policy, including the Thrift presence 
bit, before checking row-fetch format support. Added tests comparing both 
policies for true, false, and absent options across Hive/Iceberg/TVF Parquet 
and ORC, plus unsupported-format fallback. The original dispatch failed the 
regression assertions; all 280 focused ASAN tests now pass.



##########
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:
   Fixed in 6a64a67fee. Every Parquet chunk-prefetch entry point now treats a 
present row_ids request as selective, including the deferred prefetch after the 
first surviving batch. Row-ID requests also bypass whole-chunk MergeRange 
setup. Demand reads keep their query IOContext accounting.
   
   The new cached-remote test uses a real file cache and prefetch thread pool. 
Before the fix, requesting one row downloaded the entire roughly 4 MiB fixture 
while query source counters recorded only 48 KiB. The regression now passes: 
downloads stay below half the file size, dry-run download bytes are zero, and 
query source-byte counters remain populated. Source counters retain their 
existing copied-byte semantics, excluding block-alignment padding. All 280 
focused ASAN tests pass.



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