[GitHub] [arrow-rs] Dandandan commented on a change in pull request #1248: Specialized filter kernels

[GitBox <gi...@apache.org>]

Dandandan commented on a change in pull request #1248:
URL: https://github.com/apache/arrow-rs/pull/1248#discussion_r800045171



##########
File path: arrow/src/compute/kernels/filter.rs
##########
@@ -185,79 +287,580 @@ pub fn prep_null_mask_filter(filter: &BooleanArray) -> BooleanArray {
 /// # Ok(())
 /// # }
 /// ```
-pub fn filter(array: &dyn Array, predicate: &BooleanArray) -> Result<ArrayRef> {
-    if predicate.null_count() > 0 {
-        // this greatly simplifies subsequent filtering code
-        // now we only have a boolean mask to deal with
-        let predicate = prep_null_mask_filter(predicate);
-        return filter(array, &predicate);
+pub fn filter(values: &dyn Array, predicate: &BooleanArray) -> Result<ArrayRef> {
+    let predicate = FilterBuilder::new(predicate).build();
+    filter_array(values, &predicate)
+}
+
+/// Returns a new [RecordBatch] with arrays containing only values matching the filter.
+pub fn filter_record_batch(
+    record_batch: &RecordBatch,
+    predicate: &BooleanArray,
+) -> Result<RecordBatch> {
+    let filter = FilterBuilder::new(predicate).optimize().build();
+
+    let filtered_arrays = record_batch
+        .columns()
+        .iter()
+        .map(|a| filter_array(a, &filter))

Review comment:
       Does it make sense to include a condition for when we have 1 column to filter (and use the normal `filter` function instead).
   I made a PR to arrow2 some time ago as it saves some unnecessary work for when we only need to filter a single column.




-- 
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: github-unsubscribe@arrow.apache.org

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