mbutrovich commented on code in PR #11055:
URL: https://github.com/apache/arrow-rs/pull/11055#discussion_r4299233227
##########
arrow-select/src/filter.rs:
##########
@@ -737,6 +737,61 @@ where
RunArray::try_new(&run_ends, &values)
}
+/// Extract bits from `src` at positions where `filter` has a 1, packed
densely,
+/// processing 64 filter bits at a time
+fn gather_bits(src: &BooleanBuffer, filter: &BooleanBuffer, count: usize) ->
Buffer {
Review Comment:
Is `gather_bits` still reachable after #10136?
[`filter_bits`](https://github.com/apache/arrow-rs/blob/946cfbc2ee60d85abc83673187ed146228750933/arrow-select/src/filter.rs#L797-L811)
sends every filter to `filter_bits_compress` unless `count >= len - len / 64`,
or the strategy is `Indices` and `count <= len / 64`. That leaves two ways in:
- The `IndexIterator` arm needs `count >= len - len / 64`. But
[`default_strategy`](https://github.com/apache/arrow-rs/blob/946cfbc2ee60d85abc83673187ed146228750933/arrow-select/src/filter.rs#L400-L405)
only picks `IndexIterator` at a selectivity of 0.8 or less, so it never gets
here.
- The `Indices` arm already has `count <= len / 64` from the dispatcher. The
new `count * 64 >= len` check then holds only when `count * 64 == len` exactly.
To check, I added a panic at the top of `gather_bits` and ran `cargo test -p
arrow-select --lib`. Only `test_filter_bits` reached it, because it calls
`filter_bits_strategy` directly. The other 451 tests passed. None of the
`filter_kernels` or `filter_bits` benches reached it either (`cargo bench --
--test`).
`gather_bits` also packs words the same way `Packer` does in
`filter_bits_compress`. Should we drop it and leave `filter_bits_strategy` as
it is on `main`?
--
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]
