You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/08/30 15:30:45 UTC
(datafusion) branch main updated: IN LIST: use runtime dispatch for `FixedSizeBinary` filter (reduce binary size) (#24687)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new e4cf35cbca IN LIST: use runtime dispatch for `FixedSizeBinary` filter (reduce binary size) (#24687)
e4cf35cbca is described below
commit e4cf35cbca92e6fda855d5eac4104b47e0e600e9
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Sun Aug 30 15:19:55 2026 +0000
IN LIST: use runtime dispatch for `FixedSizeBinary` filter (reduce binary size) (#24687)
## Which issue does this PR close?
- Part of https://github.com/apache/datafusion/issues/24727
- Part of https://github.com/apache/datafusion/pull/24102
- Follow on to
https://github.com/apache/datafusion/pull/24102#discussion_r3851941679
## Rationale for this change
Keeping binary code size for datafusion lower is nice for many targets,
like WASM for example, that has to be downloaded. Also since datafusion
based systems are likely downloaded many times decreasing the binary
size is likely to cumulatively reduce a large number of bits being
transferred
In
https://github.com/apache/datafusion/pull/24102#discussion_r3851941679 I
wondered if the generic `FixedSizeBinaryFilter<T>` is worth the extra
generated code: it is monomorphized (aka the compiler makes 5 copies)
for 5 different widths, and each instantiation basically just does type
dispatch.
I think we can get the same performance with less (binary and source)
code.
## What changes are included in this PR?
- Remove the `T: ArrowPrimitiveType` parameter from
`FixedSizeBinaryFilter`, replacing the 5 monomorphized filter
instantiations with a single runtime `match` on `value_size()` in a new
`reinterpret` helper, called once per probed batch.
## Are these changes tested?
Yes, by the existing tests in in CI
## Are there any user-facing changes?
No
---------
Co-authored-by: Jay Zhan <ja...@gmail.com>
---
.../in_list/fixed_size_binary_filter.rs | 70 +++++++++++-----------
1 file changed, 36 insertions(+), 34 deletions(-)
diff --git a/datafusion/physical-expr/src/expressions/in_list/fixed_size_binary_filter.rs b/datafusion/physical-expr/src/expressions/in_list/fixed_size_binary_filter.rs
index ddad5a7099..b0e86cf932 100644
--- a/datafusion/physical-expr/src/expressions/in_list/fixed_size_binary_filter.rs
+++ b/datafusion/physical-expr/src/expressions/in_list/fixed_size_binary_filter.rs
@@ -38,7 +38,6 @@
//! Reinterpreting an aligned Arrow buffer is zero-copy. An unaligned buffer is
//! copied into aligned primitive storage before filter construction or probing.
-use std::marker::PhantomData;
use std::mem::size_of;
use std::sync::Arc;
@@ -84,16 +83,12 @@ where
}
/// Adapts a primitive filter to concrete, same-width `FixedSizeBinary` arrays.
-struct FixedSizeBinaryFilter<T: ArrowPrimitiveType> {
+struct FixedSizeBinaryFilter {
data_type: DataType,
inner: StaticFilterRef,
- _marker: PhantomData<T>,
}
-impl<T> StaticFilter for FixedSizeBinaryFilter<T>
-where
- T: ArrowPrimitiveType + Send + Sync + 'static,
-{
+impl StaticFilter for FixedSizeBinaryFilter {
fn null_count(&self) -> usize {
self.inner.null_count()
}
@@ -112,26 +107,28 @@ where
self.data_type
)
})?;
- let primitive = reinterpret_as_primitive::<T>(array)?;
- self.inner.contains(&primitive, negated)
+ let primitive = reinterpret(array)?.ok_or_else(|| {
+ // Instantiation code rejects unsupported widths, so this is unexpected
+ internal_datafusion_err!(
+ "FixedSizeBinary filter: unsupported width {}",
+ array.value_size()
+ )
+ })?;
+ self.inner.contains(primitive.as_ref(), negated)
}
}
-fn instantiate_for_primitive<T>(array: &FixedSizeBinaryArray) -> Result<StaticFilterRef>
-where
- T: ArrowPrimitiveType + Send + Sync + 'static,
-{
- let primitive: ArrayRef = Arc::new(reinterpret_as_primitive::<T>(array)?);
- let inner = instantiate_primitive_filter(&primitive)?.ok_or_else(|| {
- internal_datafusion_err!(
- "FixedSizeBinary filter: no primitive filter for {}",
- primitive.data_type()
- )
- })?;
- Ok(Arc::new(FixedSizeBinaryFilter::<T> {
- data_type: array.data_type().clone(),
- inner,
- _marker: PhantomData,
+/// Reinterprets a supported-width array as its same-width primitive array, or
+/// `None` if the width has no primitive representation.
+fn reinterpret(array: &FixedSizeBinaryArray) -> Result<Option<ArrayRef>> {
+ Ok(Some(match array.value_size() {
+ 1 => Arc::new(reinterpret_as_primitive::<UInt8Type>(array)?) as ArrayRef,
+ 2 => Arc::new(reinterpret_as_primitive::<UInt16Type>(array)?),
+ 4 => Arc::new(reinterpret_as_primitive::<UInt32Type>(array)?),
+ 8 => Arc::new(reinterpret_as_primitive::<UInt64Type>(array)?),
+ 16 => Arc::new(reinterpret_as_primitive::<Decimal128Type>(array)?),
+
+ _ => return Ok(None),
}))
}
@@ -139,22 +136,27 @@ where
pub(super) fn instantiate_fixed_size_binary_filter(
in_array: &ArrayRef,
) -> Result<Option<StaticFilterRef>> {
- let DataType::FixedSizeBinary(width) = in_array.data_type() else {
+ if !matches!(in_array.data_type(), DataType::FixedSizeBinary(_)) {
return Ok(None);
- };
+ }
let Some(array) = in_array.as_fixed_size_binary_opt() else {
return Ok(None);
};
- let filter = match width {
- 1 => instantiate_for_primitive::<UInt8Type>(array)?,
- 2 => instantiate_for_primitive::<UInt16Type>(array)?,
- 4 => instantiate_for_primitive::<UInt32Type>(array)?,
- 8 => instantiate_for_primitive::<UInt64Type>(array)?,
- 16 => instantiate_for_primitive::<Decimal128Type>(array)?,
- _ => return Ok(None),
+ let Some(primitive) = reinterpret(array)? else {
+ return Ok(None);
};
- Ok(Some(filter))
+ let inner = instantiate_primitive_filter(&primitive)?.ok_or_else(|| {
+ // reinterpret should have returned None for unsupported widths, so this is unexpected
+ internal_datafusion_err!(
+ "FixedSizeBinary filter: no primitive filter for {}",
+ primitive.data_type()
+ )
+ })?;
+ Ok(Some(Arc::new(FixedSizeBinaryFilter {
+ data_type: in_array.data_type().clone(),
+ inner,
+ })))
}
#[cfg(test)]
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org