haohuaijin commented on code in PR #10443:
URL: https://github.com/apache/arrow-rs/pull/10443#discussion_r3672883874


##########
parquet/src/arrow/arrow_reader/selection/boolean.rs:
##########
@@ -92,6 +93,22 @@ impl MaskSelection {
             .get_or_init(|| mask_to_selectors(&self.mask))
             .as_slice()
     }
+
+    /// Borrows the cached RLE form, converting into a temporary if not cached.
+    pub(super) fn borrowed_selectors(&self) -> Cow<'_, [RowSelector]> {

Review Comment:
   mabye we can keep the `borrowed_selectors` do not set the `self.selectors` 
cache for now. and do it later if we really need. the 
[`borrowed_selectors`](https://github.com/apache/arrow-rs/blob/e5f4ec70fefc0db102c7a7ef77eed69d806a300e/parquet/src/arrow/arrow_reader/selection/boolean.rs#L98-L104)
 is an internal helper, and its only callers are the mixed-representation paths 
in [`intersection` and 
`union`](https://github.com/apache/arrow-rs/blob/e5f4ec70fefc0db102c7a7ef77eed69d806a300e/parquet/src/arrow/arrow_reader/selection/mod.rs#L480-L516).
 and in current datafusion, the relevant call path is:
   
   1. 
[`prune_plan_with_page_index_and_metrics`](https://github.com/apache/datafusion/blob/ef94a85b85bfc19042cad8c732ad1d78f07b34a5/datafusion/datasource-parquet/src/page_filter.rs#L318-L341)
   2. 
[`ParquetAccessPlan::scan_selection`](https://github.com/apache/datafusion/blob/ef94a85b85bfc19042cad8c732ad1d78f07b34a5/datafusion/datasource-parquet/src/access_plan.rs#L389-L397)
   3. 
[`existing_selection.intersection(&selection)`](https://github.com/apache/datafusion/blob/ef94a85b85bfc19042cad8c732ad1d78f07b34a5/datafusion/datasource-parquet/src/access_plan.rs#L394-L395)
   
   After the intersection, the existing mask is immediately replaced with the 
new selection, so the cache will not be used.
   
   so, keeping the conversion temporary on a cache miss therefore seems more 
suitable for the current usage.



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

Reply via email to