Re: [PR] perf(vindex): parallelize Parquet reads for index builds [paimon-rust]

["shyjsarah (via GitHub)" <gi...@apache.org>]

shyjsarah commented on code in PR #736:
URL: https://github.com/apache/paimon-rust/pull/736#discussion_r3868620331


##########
crates/paimon/src/arrow/format/parquet.rs:
##########
@@ -490,29 +490,38 @@ impl FormatFileReader for ParquetFormatReader {
         // preserving positional `_ROW_ID`, sort order, and batch backpressure. Reads
         // with predicates or an explicit row selection retain the original
         // single-stream path until their selections are split per row group.
-        let row_group_parallelism = self
-            .read_budget
-            .as_ref()
-            .filter(|_| preds.is_empty() && row_filter_factory.is_none() && row_selection.is_none())
+        let read_budget = self.read_budget.as_ref().filter(|_| {
+            preds.is_empty() && row_filter_factory.is_none() && row_selection.is_none()
+        });
+        let row_group_parallelism = read_budget
             .map(|budget| {
                 budget
                     .parallelism()
                     .min(batch_stream_builder.metadata().num_row_groups())
             })
             .unwrap_or(1);
+        let projected_bytes = self
+            .read_budget
+            .as_ref()
+            .filter(|budget| row_group_parallelism > 1 || budget.diagnostics_enabled())
+            .map(|budget| {
+                let projected_bytes = batch_stream_builder
+                    .metadata()
+                    .row_groups()
+                    .iter()
+                    .map(|row_group| projected_row_group_bytes(row_group, &mask))
+                    .collect::<Vec<_>>();
+                budget.record_projected_row_groups(&projected_bytes);

Review Comment:
   Non-blocking: this avoids the previous all-zero diagnostics for selected reads, but it now records metadata for **all** row groups in the file rather than only the row groups touched by the effective selection. The added test selects rows `0..9` from a two-row-group file and expects `row_group_count == 2`, even though only the first row group is involved. As a result, `parquet_row_group_count` and the projected-byte min/max/total can overstate the work performed by partial reads.
   
   Could we either filter these values by the effective `RowSelection`, or rename/document them as file-level projected metadata rather than read-level diagnostics?



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org