Skip to content

Commit f723412

Browse files
authored
fix: Adjust the constant optimization in list_contains (#3211)
The constant optimization in `list_contains` was incorrect (and could cause a crash like #3210), because it: 1. produced a `ConstantArray` with the length of the `elements` array, rather than with the length of the input `ListArray`. 2. produced a `ConstantArray(true)` when all `elements` matched, without correlating with the `offsets` (which might contain empty lists). This change adjusts the constant optimization to only trigger if no elements match, and to produce a `ConstantArray` of the appropriate length. Fixes #3210.
1 parent 8f1ad34 commit f723412

File tree

1 file changed

+22
-5
lines changed

1 file changed

+22
-5
lines changed

vortex-array/src/compute/list.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,12 @@ pub fn list_contains(array: &dyn Array, value: Scalar) -> VortexResult<ArrayRef>
6262
let matching_elements = compare(elems, &rhs, Operator::Eq)?;
6363
let matches = matching_elements.to_bool()?;
6464

65-
// Fast path: all elements match or none match.
65+
// Fast path: no elements match.
6666
if let Some(pred) = matches.as_constant() {
67-
return match pred.as_bool().value() {
67+
if matches!(pred.as_bool().value(), None | Some(false)) {
6868
// TODO(aduffy): how do we handle null?
69-
None | Some(false) => Ok(ConstantArray::new::<bool>(false, matches.len()).into_array()),
70-
Some(true) => Ok(ConstantArray::new::<bool>(true, matches.len()).into_array()),
71-
};
69+
return Ok(ConstantArray::new::<bool>(false, list_array.len()).into_array());
70+
}
7271
}
7372

7473
match_each_integer_ptype!(ends.ptype(), |$T| {
@@ -251,6 +250,24 @@ mod tests {
251250
None,
252251
bool_array(vec![false, true, true], None)
253252
)]
253+
// Case 3: list(utf8) with all elements matching, but some empty lists
254+
#[case(
255+
nonnull_strings(vec![vec![], vec!["a"], vec!["a"]]),
256+
Some("a"),
257+
bool_array(vec![false, true, true], None)
258+
)]
259+
// Case 4: list(utf8) all lists empty.
260+
#[case(
261+
nonnull_strings(vec![vec![], vec![], vec![]]),
262+
Some("a"),
263+
bool_array(vec![false, false, false], None)
264+
)]
265+
// Case 5: list(utf8) no elements matching.
266+
#[case(
267+
nonnull_strings(vec![vec!["b"], vec![], vec!["b"]]),
268+
Some("a"),
269+
bool_array(vec![false, false, false], None)
270+
)]
254271
fn test_contains_nullable(
255272
#[case] list_array: ArrayRef,
256273
#[case] value: Option<&str>,

0 commit comments

Comments
 (0)