Re: [PR] IN LIST: use runtime dispatch for `FixedSizeBinary` filter (reduce binary size) [datafusion]

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #24687:
URL: https://github.com/apache/datafusion/pull/24687#discussion_r3862670213


##########
datafusion/physical-expr/src/expressions/in_list/fixed_size_binary_filter.rs:
##########
@@ -84,16 +83,12 @@ where
 }
 
 /// Adapts a primitive filter to concrete, same-width `FixedSizeBinary` arrays.
-struct FixedSizeBinaryFilter<T: ArrowPrimitiveType> {
+struct FixedSizeBinaryFilter {

Review Comment:
   the point of this PR is to remove this template type, so there is only one copy of this code rather than 5



##########
datafusion/physical-expr/src/expressions/in_list/fixed_size_binary_filter.rs:
##########


Review Comment:
   @geoffreyclaude points out [here](https://github.com/apache/datafusion/pull/24102#discussion_r3854450966) that a lot of the code size savings are due to the `handle_dictionary!` expansion which makes `::contains` substantially larger
   
   After we merge this PR, the code size savings will be smaller
   - https://github.com/apache/datafusion/pull/24662
   
   However, I think the savings will still be real, so this PR is worth pursuing still
   
   



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org