adriangb opened a new issue, #24709: URL: https://github.com/apache/datafusion/issues/24709
Follow-up from #24526, tracking issue. That PR added a compact sorted-domain representation for large `IN` lists, limited to string columns with all-string literals. The review discussion suggested there's no fundamental reason for that limit — the underlying idea (sorted deduplicated domain, binary search for the first value at or above `min`, one comparison against `max`) works for any total order. This issue is for figuring out what "more types" should mean and what each family needs. **Type families, and what makes them different** - `Binary` / `LargeBinary` / `BinaryView` — byte comparison, same as strings; `DataType::is_string()` just doesn't include them. Probably the smallest step, and shares whatever ordering caveats #24525 addressed for byte arrays. - Int/UInt 8–64, plus the types that are physically integers (`Date32`/`Date64`, `Time32`/`Time64`, `Timestamp`, `Duration`) — ordering is unambiguous, so the question is mostly representation. - `Decimal128` / `Decimal256` — mechanically similar, with the added question of precision/scale agreement between literals and statistics. - `Float32` / `Float64` — NaN and ±0.0 don't fit a naive total order. Might be better excluded explicitly with a note than supported. - Bool, Interval, nested — probably not worth it: bool is cardinality-2, interval ordering is semantically murky, and nested-field pruning isn't supported anyway. **Representation** This probably wants settling before the per-type work, since it decides whether this is one change or several. The current implementation stores `Arc<[String]>` and compares `&[u8]`. Options raised in review: monomorphize per `ArrowPrimitiveType`; hold an Arrow array and use comparison kernels; or encode through an order-preserving byte format such as `arrow::row::RowConverter` so one `&[u8]` implementation covers every orderable type. The last is available without a new dependency (`datafusion-pruning` already depends on `arrow`) and would also fold in the separate question of whether `Arc<[String]>` is the right layout — `Rows` is contiguous bytes plus offsets, which removes the per-probe pointer chase. But none of these have been measured, and the per-batch encode cost would need checking against the existing `Utf8View` cast before assuming it's free. **Two related items that may belong here rather than separately** - Eligibility is gated on the pre-dedup list length, while the compact form's cost is closer to O(distinct). Dedup-then-check might let heavily-duplicated lists qualify at a lower cap. - `NOT IN` never reaches the compact path, so a large `NOT IN` under a raised cap still expands into a long AND chain. Capping `NOT IN` expansion separately from the config value could be worth considering as part of whatever cap policy falls out of this. Types that stay uncovered could potentially fall back to an enclosing-range form instead of no pruning at all. -- 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]
