This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 784fa7c2ad chore: deduplicate filter nulls code in coalesce/filter
kernel (#10348)
784fa7c2ad is described below
commit 784fa7c2ad06aac24686285228d9dec49eed204d
Author: Jeffrey Vo <[email protected]>
AuthorDate: Sun Jul 26 07:20:31 2026 +0900
chore: deduplicate filter nulls code in coalesce/filter kernel (#10348)
whilst reviewing #10236 i noticed we have a little code duplication, so
remedying that
---
arrow-select/src/coalesce/byte_view.rs | 16 ++++++----------
arrow-select/src/coalesce/primitive.rs | 14 +++-----------
arrow-select/src/filter.rs | 27 ---------------------------
3 files changed, 9 insertions(+), 48 deletions(-)
diff --git a/arrow-select/src/coalesce/byte_view.rs
b/arrow-select/src/coalesce/byte_view.rs
index f43572b37c..4cf0e006df 100644
--- a/arrow-select/src/coalesce/byte_view.rs
+++ b/arrow-select/src/coalesce/byte_view.rs
@@ -16,11 +16,11 @@
// under the License.
use crate::coalesce::InProgressArray;
-use crate::filter::{FilterPredicate, FilterSelection, filter_null_mask};
+use crate::filter::{FilterPredicate, FilterSelection};
use arrow_array::cast::AsArray;
use arrow_array::types::ByteViewType;
use arrow_array::{Array, ArrayRef, GenericByteViewArray};
-use arrow_buffer::{BooleanBuffer, Buffer, NullBuffer, NullBufferBuilder};
+use arrow_buffer::{Buffer, NullBuffer, NullBufferBuilder};
use arrow_data::{ByteView, MAX_INLINE_VIEW_LEN};
use arrow_schema::ArrowError;
use std::marker::PhantomData;
@@ -165,15 +165,11 @@ impl<B: ByteViewType> InProgressByteViewArray<B> {
filter: &FilterPredicate,
source_nulls: Option<&NullBuffer>,
) {
- let Some((null_count, nulls)) = filter_null_mask(source_nulls, filter)
else {
+ if let Some(nulls) = filter.filter_nulls(source_nulls) {
+ self.nulls.append_buffer(&nulls);
+ } else {
self.nulls.append_n_non_nulls(filter.count());
- return;
- };
-
- let nulls = unsafe {
- NullBuffer::new_unchecked(BooleanBuffer::new(nulls, 0,
filter.count()), null_count)
- };
- self.nulls.append_buffer(&nulls);
+ }
}
/// Append views to self.views, updating the buffer index if necessary
diff --git a/arrow-select/src/coalesce/primitive.rs
b/arrow-select/src/coalesce/primitive.rs
index 21ddd956cc..3fa070d627 100644
--- a/arrow-select/src/coalesce/primitive.rs
+++ b/arrow-select/src/coalesce/primitive.rs
@@ -16,12 +16,10 @@
// under the License.
use crate::coalesce::InProgressArray;
-use crate::filter::{
- FilterIndices, FilterPredicate, FilterSelection, FilterSlices,
filter_null_mask,
-};
+use crate::filter::{FilterIndices, FilterPredicate, FilterSelection,
FilterSlices};
use arrow_array::cast::AsArray;
use arrow_array::{Array, ArrayRef, ArrowPrimitiveType, PrimitiveArray};
-use arrow_buffer::{BooleanBuffer, NullBuffer, NullBufferBuilder, ScalarBuffer};
+use arrow_buffer::{NullBuffer, NullBufferBuilder, ScalarBuffer};
use arrow_schema::{ArrowError, DataType};
use std::fmt::Debug;
use std::sync::Arc;
@@ -140,13 +138,7 @@ fn append_filtered_nulls(
source_nulls: Option<&NullBuffer>,
filter: &FilterPredicate,
) {
- if let Some((null_count, filtered_nulls)) = filter_null_mask(source_nulls,
filter) {
- let filtered_nulls = unsafe {
- NullBuffer::new_unchecked(
- BooleanBuffer::new(filtered_nulls, 0, filter.count()),
- null_count,
- )
- };
+ if let Some(filtered_nulls) = filter.filter_nulls(source_nulls) {
nulls.append_buffer(&filtered_nulls);
} else {
nulls.append_n_non_nulls(filter.count());
diff --git a/arrow-select/src/filter.rs b/arrow-select/src/filter.rs
index 8b69811d4a..b7227cf6d9 100644
--- a/arrow-select/src/filter.rs
+++ b/arrow-select/src/filter.rs
@@ -660,33 +660,6 @@ where
RunArray::try_new(&run_ends, &values)
}
-/// Computes a new null mask for `data` based on `predicate`
-///
-/// If the predicate selected no null-rows, returns `None`, otherwise returns
-/// `Some((null_count, null_buffer))` where `null_count` is the number of nulls
-/// in the filtered output, and `null_buffer` is the filtered null buffer
-///
-pub(crate) fn filter_null_mask(
- nulls: Option<&NullBuffer>,
- predicate: &FilterPredicate,
-) -> Option<(usize, Buffer)> {
- let nulls = nulls?;
- if nulls.null_count() == 0 {
- return None;
- }
-
- let nulls = filter_bits(nulls.inner(), predicate);
- // The filtered `nulls` has a length of `predicate.count` bits and
- // therefore the null count is this minus the number of valid bits
- let null_count = predicate.count - nulls.count_set_bits_offset(0,
predicate.count);
-
- if null_count == 0 {
- return None;
- }
-
- Some((null_count, nulls))
-}
-
/// Filter the packed bitmask `buffer`, with `predicate` starting at bit
offset `offset`
fn filter_bits(buffer: &BooleanBuffer, predicate: &FilterPredicate) -> Buffer {
let src = buffer.values();
