diff --git a/vortex-array/src/expr/transform/match_between.rs b/vortex-array/src/expr/transform/match_between.rs index 7cb427596f4..0d5ebc66923 100644 --- a/vortex-array/src/expr/transform/match_between.rs +++ b/vortex-array/src/expr/transform/match_between.rs @@ -19,18 +19,31 @@ use crate::scalar_fn::fns::operators::Operator; pub fn find_between(expr: Expression) -> Expression { // We search all pairs of cnfs to find any pair of expressions can be converted into a between // expression. - let mut conjuncts = conjuncts(&expr); + // + // Only leaf conjuncts (`GetItem op Literal`) can form a between; anything else is moved to the + // rest unexamined so pairs of non-leaf conjuncts are never compared. + let mut conjuncts = conjuncts(&expr) + .into_iter() + .map(|expression| { + let is_leaf = is_leaf_conjunct(&expression); + (expression, is_leaf) + }) + .collect::>(); let mut rest = vec![]; for idx in 0..conjuncts.len() { - let Some(c) = conjuncts.get(idx).cloned() else { + let Some((c, is_leaf)) = conjuncts.get(idx).cloned() else { continue; }; + if !is_leaf { + rest.push(c); + continue; + } let mut matched = false; for idx2 in (idx + 1)..conjuncts.len() { // Since values are removed in iterations there might not be a value at idx2, // but all values will have been considered. - let Some(c2) = conjuncts.get(idx2) else { + let Some((c2, true)) = conjuncts.get(idx2) else { continue; }; if let Some(expr) = maybe_match(&c, c2) { @@ -48,6 +61,21 @@ pub fn find_between(expr: Expression) -> Expression { and_collect(rest).unwrap_or_else(|| lit(true)) } +/// A leaf conjunct is a strict-comparison binary between a `GetItem` and a `Literal`, in either +/// order — the only shape that can be half of a between. +fn is_leaf_conjunct(expr: &Expression) -> bool { + let Some(operator) = expr.as_opt::() else { + return false; + }; + if is_strict_comparison(*operator).is_none() { + return false; + } + + let lhs = expr.child(0); + let rhs = expr.child(1); + (lhs.is::() && rhs.is::()) || (rhs.is::() && lhs.is::()) +} + fn maybe_match(lhs: &Expression, rhs: &Expression) -> Option { let (Some(lhs_op), Some(rhs_op)) = (lhs.as_opt::(), rhs.as_opt::()) else { return None; @@ -332,4 +360,32 @@ mod tests { &find ); } + + #[test] + fn test_match_between_skips_non_leaf_conjuncts() { + let lower = gt_eq(col("x"), lit(2)); + let non_leaf = gt(col("y"), col("z")); + let upper = lt(col("x"), lit(5)); + assert!(super::is_leaf_conjunct(&lower)); + assert!(!super::is_leaf_conjunct(&non_leaf)); + assert!(super::is_leaf_conjunct(&upper)); + + let find = find_between(and(and(lower, non_leaf.clone()), upper)); + + assert_eq!( + &and( + between( + col("x"), + lit(2), + lit(5), + BetweenOptions { + lower_strict: StrictComparison::NonStrict, + upper_strict: StrictComparison::Strict, + } + ), + non_leaf, + ), + &find + ); + } }