Skip to content

Commit 2c939fe

Browse files
authored
Fix pruning expressions with and / or (#2595)
1 parent 442d6df commit 2c939fe

File tree

3 files changed

+35
-4
lines changed

3 files changed

+35
-4
lines changed

vortex-array/src/compute/between.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ where
5454
}
5555
}
5656

57-
/// Compute between (a <= x <= b), this can be implemented using compare and boolean andn but this
57+
/// Compute between (a <= x <= b), this can be implemented using compare and boolean and but this
5858
/// will likely have a lower runtime.
5959
///
6060
/// This semantics is equivalent to:

vortex-expr/src/operators.rs

+8
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ impl Operator {
4747
}
4848
}
4949

50+
pub fn logical_inverse(self) -> Option<Self> {
51+
match self {
52+
Operator::And => Some(Operator::Or),
53+
Operator::Or => Some(Operator::And),
54+
_ => None,
55+
}
56+
}
57+
5058
/// Change the sides of the operator, where changing lhs and rhs won't change the result of the operation
5159
pub fn swap(self) -> Self {
5260
match self {

vortex-expr/src/pruning.rs

+26-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use vortex_array::aliases::hash_set::HashSet;
1010
use vortex_array::stats::Stat;
1111
use vortex_array::{Array, ArrayRef};
1212
use vortex_dtype::{FieldName, Nullability};
13-
use vortex_error::{VortexExpect as _, VortexResult};
13+
use vortex_error::{VortexExpect, VortexResult};
1414
use vortex_scalar::Scalar;
1515

1616
use crate::between::Between;
@@ -184,8 +184,12 @@ fn convert_to_pruning_expression(expr: &ExprRef) -> PruningPredicateStats {
184184
let (rewritten_left, mut refs_lhs) = convert_to_pruning_expression(bexp.lhs());
185185
let (rewritten_right, refs_rhs) = convert_to_pruning_expression(bexp.rhs());
186186
refs_lhs.extend(refs_rhs);
187+
let flipped_op = bexp
188+
.op()
189+
.logical_inverse()
190+
.vortex_expect("Can not be any other operator than and / or");
187191
return (
188-
BinaryExpr::new_expr(rewritten_left, bexp.op(), rewritten_right),
192+
BinaryExpr::new_expr(rewritten_left, flipped_op, rewritten_right),
189193
refs_lhs,
190194
);
191195
}
@@ -705,10 +709,29 @@ mod tests {
705709
let predicate = PruningPredicate::try_new(&expr).unwrap();
706710
assert_eq!(predicate.required_stats(), &expected);
707711

708-
let expected_expr = or(
712+
let expected_expr = and(
709713
gt_eq(get_item_scope(FieldName::from("min")), lit(10)),
710714
lt_eq(get_item_scope(FieldName::from("max")), lit(50)),
711715
);
712716
assert_eq!(predicate.expr(), &expected_expr)
713717
}
718+
#[test]
719+
pub fn pruning_and_or_operators() {
720+
// Test case: a > 10 AND a < 50
721+
let column = FieldName::from("a");
722+
let and_expr = and(
723+
gt(get_item_scope(column.clone()), lit(10)),
724+
lt(get_item_scope(column), lit(50)),
725+
);
726+
let pruned = PruningPredicate::try_new(&and_expr).unwrap();
727+
728+
// Expected: a_max <= 10 OR a_min >= 50
729+
assert_eq!(
730+
pruned.expr(),
731+
&or(
732+
lt_eq(get_item_scope(FieldName::from("a_max")), lit(10)),
733+
gt_eq(get_item_scope(FieldName::from("a_min")), lit(50))
734+
),
735+
);
736+
}
714737
}

0 commit comments

Comments
 (0)