This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11378-b98f76beab88301fecb00afe1e2cbc306befed79 in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit ef3c950c1c17a552bf146d841da50ae01536d675 Author: Liang-Chi Hsieh <[email protected]> AuthorDate: Mon Oct 5 01:12:51 2026 +0000 Fix filter_nulls for predicates selecting all or no rows (#11378) # Which issue does this PR close? Closes #11377. # Rationale for this change Direct calls to the public `FilterPredicate::filter_nulls` panic when an input contains nulls and the predicate selects all rows or no rows. Both strategies reach an `unreachable!()` in `filter_bits`. # What changes are included in this PR? Handle both strategies in `filter_nulls`: return `None` for an empty selection, and slice the input to the predicate's length for an all-row selection, returning `None` if the slice has no nulls. Preserve and document the existing length check for inputs containing nulls. # Are these changes tested? Added regressions that reproduced both panics before the fix, plus coverage for shorter predicates, a selected prefix without nulls, nonzero bitmap offsets, empty predicates, general iteration strategies with and without optimization, and existing length-check behavior. Validation (Cargo builds and tests used the company crates proxy): - `cargo test -p arrow-select`: 432 unit tests and 17 doc tests passed. - `cargo clippy -p arrow-select --all-targets --all-features -- -D warnings`: passed. - `cargo +stable fmt --all -- --check` and `git diff --check`: passed. # Are there any user-facing changes? Valid all-row and no-row predicates no longer panic in `filter_nulls`. No public signatures change. AI assistance was used to investigate the defect, implement the fix, write regression tests, and draft this description. The resulting diff was reviewed and the checks above were run. --- arrow-select/src/filter.rs | 88 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/arrow-select/src/filter.rs b/arrow-select/src/filter.rs index 9baccf745c..5767f4a18c 100644 --- a/arrow-select/src/filter.rs +++ b/arrow-select/src/filter.rs @@ -556,12 +556,26 @@ impl FilterPredicate { /// because the input `nulls` was `None`, the input had no nulls, or the /// filtered result has no nulls. Otherwise returns the filtered /// [`NullBuffer`] with its precomputed null count. + /// + /// # Panics + /// + /// Panics if the input contains nulls and is shorter than this predicate. pub fn filter_nulls(&self, nulls: Option<&NullBuffer>) -> Option<NullBuffer> { let nulls = nulls?; if nulls.null_count() == 0 { return None; } + assert!(nulls.len() >= self.filter.len()); + match self.strategy { + IterationStrategy::None => return None, + IterationStrategy::All => { + let nulls = nulls.slice(0, self.count); + return (nulls.null_count() != 0).then_some(nulls); + } + _ => {} + } + let nulls = filter_bits(nulls.inner(), self); // The filtered `nulls` has a length of `self.count` bits and therefore // the null count is this minus the number of valid bits @@ -1264,6 +1278,80 @@ mod tests { use rand::prelude::*; use rand::rng; + #[test] + fn test_filter_nulls_all() { + let predicate = FilterBuilder::new(&BooleanArray::from(vec![true, true])).build(); + let nulls = NullBuffer::from(vec![true, false]); + assert_eq!(predicate.filter_nulls(Some(&nulls)), Some(nulls)); + } + + #[test] + fn test_filter_nulls_none() { + let predicate = FilterBuilder::new(&BooleanArray::from(vec![false, false])).build(); + let nulls = NullBuffer::from(vec![true, false]); + assert_eq!(predicate.filter_nulls(Some(&nulls)), None); + } + + #[test] + fn test_filter_nulls_selection() { + // Cover full and partial prefixes, empty selections, and both general + // iteration strategies, with and without materializing the selection. + let filters = [ + vec![], + vec![true], + vec![true, true], + vec![true; 6], + vec![false; 6], + vec![true, false, false, false, false, false], + vec![true, true, true, true, true, false], + ]; + for offset in [0, 3, 9] { + let mut validity = vec![false; offset]; + validity.extend([true, false, true, false, true, false]); + let nulls = NullBuffer::from(validity).slice(offset, 6); + for filter in &filters { + let expected: NullBuffer = filter + .iter() + .enumerate() + .filter(|(_, selected)| **selected) + .map(|(i, _)| nulls.is_valid(i)) + .collect(); + let expected = (expected.null_count() != 0).then_some(expected); + let filter = BooleanArray::from(filter.clone()); + for optimize in [false, true] { + let builder = FilterBuilder::new(&filter); + let predicate = if optimize { + builder.optimize() + } else { + builder + } + .build(); + assert_eq!(predicate.filter_nulls(Some(&nulls)), expected); + assert_eq!(predicate.filter_nulls(None), None); + assert_eq!( + predicate.filter_nulls(Some(&NullBuffer::new_valid(6))), + None + ); + } + } + } + } + + #[test] + fn test_filter_nulls_length_check() { + for filter in [vec![true, true], vec![false, false], vec![true, false]] { + let predicate = FilterBuilder::new(&BooleanArray::from(filter)).build(); + let nulls = NullBuffer::new_null(1); + assert!(std::panic::catch_unwind(|| predicate.filter_nulls(Some(&nulls))).is_err()); + // Inputs without nulls continue to short-circuit before checking length. + assert_eq!( + predicate.filter_nulls(Some(&NullBuffer::new_valid(1))), + None + ); + assert_eq!(predicate.filter_nulls(None), None); + } + } + macro_rules! def_temporal_test { ($test:ident, $array_type: ident, $data: expr) => { #[test]
