alamb opened a new issue, #24658:
URL: https://github.com/apache/datafusion/issues/24658
### Is your feature request related to a problem or challenge?
- Part of #19241
Every `StaticFilter` in the `in_list` module handles dictionary-encoded
needles itself, via the `handle_dictionary!` macro (or a hand-rolled copy in
`ArrayStaticFilter`). The macro expands `downcast_dictionary_array!` — a match
over all eight dictionary key types — into every monomorphized `contains`
implementation.
Measured with `cargo llvm-lines -p datafusion-physical-expr --lib` (as of
#24102): the module has 44 monomorphized `contains` copies averaging ~744 LLVM
IR lines each, of which ~85% is the repeated dictionary match. In total the
duplicated dictionary handling accounts for ~27,000 IR lines, about 4% of the
whole crate. Each new specialized filter in the #19241 series stamps out more
copies.
### Describe the solution you'd like
Implement dictionary handling once as a wrapper filter that delegates to any
inner `StaticFilter` for the values, and wrap it once in
`instantiate_static_filter`:
```rust
/// Unwraps dictionary-encoded needles for the wrapped filter.
struct DictionaryFilter {
/// The haystack's (non-dictionary) value type.
values_type: DataType,
inner: StaticFilterRef,
}
impl StaticFilter for DictionaryFilter {
fn null_count(&self) -> usize {
self.inner.null_count()
}
fn contains(&self, v: &dyn Array, negated: bool) -> Result<BooleanArray>
{
downcast_dictionary_array! {
v => {
if v.values().data_type() == &self.values_type {
let values_contains =
self.inner.contains(v.values().as_ref(), negated)?;
let result = take(&values_contains, v.keys(), None)?;
return Ok(downcast_array(result.as_ref()));
}
}
_ => {}
}
self.inner.contains(v, negated)
}
}
```
The `handle_dictionary!` macro and `ArrayStaticFilter`'s inline dictionary
block are then deleted. The type guard (only unwrap when the dictionary's value
type matches the haystack type) preserves `ArrayStaticFilter`'s existing
fallback for mismatched dictionaries; Arrow forbids nested dictionaries, so one
unwrap level suffices.
A prototype of this change compiles the dictionary machinery exactly once:
the 44 `contains` copies drop from ~744 to ~116 IR lines on average, shrinking
`datafusion-physical-expr` by ~27,450 IR lines (~4.1%) with all in_list unit
tests and sqllogictests passing. Runtime cost is one virtual call plus a
`DataType` match per batch; per-row hot loops are unchanged.
Future representation adapters (e.g. #24088) then get dictionary support for
free instead of embedding the match per instantiation.
### Describe alternatives you've considered
Outlining the macro body into a shared helper function called from each
filter. This achieves most of the same code-size win but keeps dictionary logic
as a per-filter obligation rather than composing it once at construction.
### Additional context
Discussed while reviewing #24102; the prototype applies on top of that PR.
--
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]
