geoffreyclaude commented on code in PR #23014:
URL: https://github.com/apache/datafusion/pull/23014#discussion_r3636853912
##########
datafusion/physical-expr/src/expressions/in_list/strategy.rs:
##########
@@ -20,32 +20,102 @@ use std::sync::Arc;
use arrow::array::ArrayRef;
use arrow::compute::cast;
use arrow::datatypes::{
- DataType, Float16Type, Int8Type, Int16Type, UInt8Type, UInt16Type,
+ DataType, Date32Type, Date64Type, Decimal128Type, DurationMicrosecondType,
+ DurationMillisecondType, DurationNanosecondType, DurationSecondType,
Float16Type,
+ Float32Type, Float64Type, Int8Type, Int16Type, Int32Type, Int64Type,
+ IntervalMonthDayNanoType, IntervalUnit, Time32MillisecondType,
Time32SecondType,
+ Time64MicrosecondType, Time64NanosecondType, TimeUnit,
TimestampMicrosecondType,
+ TimestampMillisecondType, TimestampNanosecondType, TimestampSecondType,
UInt8Type,
+ UInt16Type, UInt32Type, UInt64Type,
};
use datafusion_common::Result;
use super::array_static_filter::ArrayStaticFilter;
use super::primitive_filter::*;
use super::static_filter::StaticFilter;
-pub(super) fn instantiate_static_filter(
- in_array: ArrayRef,
-) -> Result<Arc<dyn StaticFilter + Send + Sync>> {
+type StaticFilterRef = Arc<dyn StaticFilter + Send + Sync>;
+
+pub(super) fn instantiate_static_filter(in_array: ArrayRef) ->
Result<StaticFilterRef> {
+ let in_array = flatten_dictionary_haystack(in_array)?;
+
+ if let Some(filter) = instantiate_branchless_filter(&in_array)? {
+ return Ok(filter);
+ }
+
+ instantiate_standard_filter(in_array)
+}
+
+fn flatten_dictionary_haystack(in_array: ArrayRef) -> Result<ArrayRef> {
// Flatten dictionary-encoded haystacks to their value type so that
// specialized filters (e.g. Int32StaticFilter) are used instead of
// falling through to the generic ArrayStaticFilter.
- let in_array = match in_array.data_type() {
- DataType::Dictionary(_, value_type) => cast(&in_array,
value_type.as_ref())?,
- _ => in_array,
- };
match in_array.data_type() {
- DataType::Int8 =>
Ok(Arc::new(BitmapFilter::<Int8Type>::try_new(&in_array)?)),
- DataType::UInt8 =>
Ok(Arc::new(BitmapFilter::<UInt8Type>::try_new(&in_array)?)),
- DataType::Int16 =>
Ok(Arc::new(BitmapFilter::<Int16Type>::try_new(&in_array)?)),
- DataType::UInt16 =>
Ok(Arc::new(BitmapFilter::<UInt16Type>::try_new(&in_array)?)),
- DataType::Float16 => {
- Ok(Arc::new(BitmapFilter::<Float16Type>::try_new(&in_array)?))
+ DataType::Dictionary(_, value_type) => Ok(cast(&in_array,
value_type.as_ref())?),
+ _ => Ok(in_array),
+ }
+}
+
+fn instantiate_branchless_filter(in_array: &ArrayRef) ->
Result<Option<StaticFilterRef>> {
+ let non_null_count = in_array.len() - in_array.null_count();
+
+ macro_rules! filter {
+ ($arrow_type:ty) => {
+ branchless_filter::<$arrow_type>(in_array, non_null_count)
+ };
+ }
+
+ match in_array.data_type() {
Review Comment:
Good point. The latest refactor removed the per-length dispatch from this
module, but it's still not super clear, I admit.
For the small types, `Int8`/`UInt8` use direct comparisons for up to 16
non-null values, and `Int16`/`UInt16`/`Float16` for up to 8. Longer lists fall
back to the bitmap filter. I added a short comment to make that explicit...
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
