alamb commented on code in PR #11206:
URL: https://github.com/apache/arrow-rs/pull/11206#discussion_r4126557324


##########
arrow-select/src/filter.rs:
##########
@@ -248,33 +254,62 @@ pub fn filter_record_batch(
 #[derive(Debug)]
 pub struct FilterBuilder {
     filter: BooleanArray,
-    count: usize,
-    strategy: IterationStrategy,
+    /// The number of rows `filter` selects, if provided by 
[`Self::with_count`]
+    count: Option<usize>,
+    optimize: bool,
 }
 
 impl FilterBuilder {
     /// Create a new [`FilterBuilder`] that can be used to construct a 
[`FilterPredicate`]
     pub fn new(filter: &BooleanArray) -> Self {
-        Self::new_with_count(filter, filter.true_count())
-    }
-
-    pub(crate) fn new_with_count(filter: &BooleanArray, count: usize) -> Self {
         let filter = match filter.null_count() {
             0 => filter.clone(),
             _ => prep_null_mask_filter(filter),
         };
 
-        let strategy = IterationStrategy::default_strategy(filter.len(), 
count);
-
         Self {
             filter,
-            count,
-            strategy,
+            count: None,
+            optimize: false,
         }
     }
 
-    /// Compute an optimized representation of the provided `filter` mask that 
can be
-    /// applied to an array more quickly.
+    /// Set the number of rows that the filter selects, so that [`Self::build`]
+    /// does not have to count them.
+    ///
+    /// Callers that build a mask row by row, or derive it from a validity
+    /// buffer with a cached null count, often already hold this number.
+    ///
+    /// # Safety
+    ///
+    /// `count` must equal [`BooleanArray::true_count`] of the filter passed to
+    /// [`Self::new`]: the number of `true` values that are not null.
+    ///
+    /// # Example
+    /// ```
+    /// # use arrow_array::{BooleanArray, Int32Array};
+    /// # use arrow_select::filter::FilterBuilder;
+    /// let values = Int32Array::from(vec![1, 2, 3, 4]);
+    /// let mask = BooleanArray::from(vec![Some(true), None, Some(true), 
Some(false)]);
+    /// // The null is not selected, so the mask selects two rows.

Review Comment:
   this is a nice example to get all three options



##########
arrow-select/src/coalesce.rs:
##########
@@ -262,7 +266,45 @@ impl BatchCoalescer {
         batch: RecordBatch,
         filter: &BooleanArray,
     ) -> Result<(), ArrowError> {
-        self.push_batch_with_filtered_columns(batch, filter)
+        self.push_batch_with_filter_builder(batch, FilterBuilder::new(filter))
+    }
+
+    /// Push a batch into the Coalescer after applying the filter described by
+    /// `filter_builder`.
+    ///
+    /// This is [`Self::push_batch_with_filter`] for a [`FilterBuilder`] the
+    /// caller has already created. For example, callers that already know how
+    /// many rows the filter selects can provide that number with
+    /// [`FilterBuilder::with_count`] so that it is not counted again.
+    ///
+    /// Callers do not need to call [`FilterBuilder::optimize`]: like
+    /// [`Self::push_batch_with_filter`], this optimizes the filter when 
`batch`
+    /// has more than one column, or one column for which
+    /// [`FilterBuilder::is_optimize_beneficial`] returns true. A filter the
+    /// caller already optimized stays optimized.
+    ///
+    /// # Example

Review Comment:
   ❤️ 



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