Skip to content

Commit 9dde184

Browse files
committed
fix
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent 86b48b5 commit 9dde184

6 files changed

Lines changed: 36 additions & 25 deletions

File tree

encodings/datetime-parts/src/compute/compare.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ use vortex_array::arrays::ConstantArray;
88
use vortex_array::compute::CompareKernel;
99
use vortex_array::compute::CompareKernelAdapter;
1010
use vortex_array::compute::Operator;
11-
use vortex_array::compute::and;
11+
use vortex_array::compute::and_kleene;
1212
use vortex_array::compute::cast;
1313
use vortex_array::compute::compare;
14-
use vortex_array::compute::or;
14+
use vortex_array::compute::or_kleene;
1515
use vortex_array::register_kernel;
1616
use vortex_dtype::DType;
1717
use vortex_dtype::Nullability;
@@ -84,7 +84,7 @@ fn compare_eq(
8484
return Ok(Some(comparison));
8585
}
8686

87-
comparison = and(
87+
comparison = and_kleene(
8888
&compare_dtp(lhs.seconds(), ts_parts.seconds, Operator::Eq, nullability)?,
8989
&comparison,
9090
)?;
@@ -94,7 +94,7 @@ fn compare_eq(
9494
return Ok(Some(comparison));
9595
}
9696

97-
comparison = and(
97+
comparison = and_kleene(
9898
&compare_dtp(
9999
lhs.subseconds(),
100100
ts_parts.subseconds,
@@ -118,7 +118,7 @@ fn compare_ne(
118118
return Ok(Some(comparison));
119119
}
120120

121-
comparison = or(
121+
comparison = or_kleene(
122122
&compare_dtp(
123123
lhs.seconds(),
124124
ts_parts.seconds,
@@ -133,7 +133,7 @@ fn compare_ne(
133133
return Ok(Some(comparison));
134134
}
135135

136-
comparison = or(
136+
comparison = or_kleene(
137137
&compare_dtp(
138138
lhs.subseconds(),
139139
ts_parts.subseconds,

vortex-array/src/compute/between.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ impl ComputeFnVTable for Between {
152152
Ok(boolean(
153153
&compare(lower, array, options.lower_strict.to_operator())?,
154154
&compare(array, upper, options.upper_strict.to_operator())?,
155-
BooleanOperator::And,
155+
BooleanOperator::AndKleene,
156156
)?
157157
.into())
158158
}

vortex-array/src/compute/boolean.rs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use crate::expr::operators::Operator;
2424
/// semantics is also known as "Bochvar logic" and "weak Kleene logic".
2525
///
2626
/// See also [BooleanOperator::And]
27+
#[deprecated(note = "Use and_kleene instead. Non-Kleene boolean ops cannot be lazily evaluated.")]
2728
pub fn and(lhs: &dyn Array, rhs: &dyn Array) -> VortexResult<ArrayRef> {
2829
boolean(lhs, rhs, BooleanOperator::And)
2930
}
@@ -41,6 +42,7 @@ pub fn and_kleene(lhs: &dyn Array, rhs: &dyn Array) -> VortexResult<ArrayRef> {
4142
/// semantics is also known as "Bochvar logic" and "weak Kleene logic".
4243
///
4344
/// See also [BooleanOperator::Or]
45+
#[deprecated(note = "Use or_kleene instead. Non-Kleene boolean ops cannot be lazily evaluated.")]
4446
pub fn or(lhs: &dyn Array, rhs: &dyn Array) -> VortexResult<ArrayRef> {
4547
boolean(lhs, rhs, BooleanOperator::Or)
4648
}
@@ -53,16 +55,19 @@ pub fn or_kleene(lhs: &dyn Array, rhs: &dyn Array) -> VortexResult<ArrayRef> {
5355
}
5456

5557
/// Point-wise logical operator between two Boolean arrays.
56-
///
57-
/// This method uses Arrow-style null propagation rather than the Kleene logic semantics. This
58-
/// semantics is also known as "Bochvar logic" and "weak Kleene logic".
5958
pub fn boolean(lhs: &dyn Array, rhs: &dyn Array, op: BooleanOperator) -> VortexResult<ArrayRef> {
60-
Ok(ScalarFnArray::try_new(
61-
ScalarFn::new(Binary, Operator::try_from(op)?),
62-
vec![lhs.to_array(), rhs.to_array()],
63-
lhs.len(),
64-
)?
65-
.into_array())
59+
match Operator::try_from(op) {
60+
Ok(expr_op) => Ok(ScalarFnArray::try_new(
61+
ScalarFn::new(Binary, expr_op),
62+
vec![lhs.to_array(), rhs.to_array()],
63+
lhs.len(),
64+
)?
65+
.into_array()),
66+
Err(_) => {
67+
tracing::trace!("non-Kleene boolean op {op:?} cannot be lazily evaluated, falling back to eager Arrow evaluation");
68+
arrow_boolean(lhs.to_array(), rhs.to_array(), op)
69+
}
70+
}
6671
}
6772

6873
/// Operations over the nullable Boolean values.
@@ -156,7 +161,7 @@ mod tests {
156161
#[case(BoolArray::from_iter([Some(true), Some(false), Some(true), Some(false)].into_iter()).into_array(),
157162
BoolArray::from_iter([Some(true), Some(true), Some(false), Some(false)].into_iter()).into_array())]
158163
fn test_or(#[case] lhs: ArrayRef, #[case] rhs: ArrayRef) {
159-
let r = or(&lhs, &rhs).unwrap();
164+
let r = or_kleene(&lhs, &rhs).unwrap();
160165

161166
let r = r.to_bool().into_array();
162167

@@ -178,7 +183,7 @@ mod tests {
178183
#[case(BoolArray::from_iter([Some(true), Some(false), Some(true), Some(false)].into_iter()).into_array(),
179184
BoolArray::from_iter([Some(true), Some(true), Some(false), Some(false)].into_iter()).into_array())]
180185
fn test_and(#[case] lhs: ArrayRef, #[case] rhs: ArrayRef) {
181-
let r = and(&lhs, &rhs).unwrap().to_bool().into_array();
186+
let r = and_kleene(&lhs, &rhs).unwrap().to_bool().into_array();
182187

183188
let v0 = r.scalar_at(0).unwrap().as_bool().value();
184189
let v1 = r.scalar_at(1).unwrap().as_bool().value();

vortex-array/src/compute/conformance/consistency.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ use crate::IntoArray;
3232
use crate::arrays::BoolArray;
3333
use crate::arrays::PrimitiveArray;
3434
use crate::compute::Operator;
35-
use crate::compute::and;
35+
use crate::compute::and_kleene;
3636
use crate::compute::cast;
3737
use crate::compute::compare;
3838
use crate::compute::invert;
3939
use crate::compute::mask;
40-
use crate::compute::or;
40+
use crate::compute::or_kleene;
4141

4242
/// Tests that filter and take operations produce consistent results.
4343
///
@@ -899,11 +899,13 @@ fn test_boolean_demorgan_consistency(array: &dyn Array) {
899899
let mask = mask.as_ref();
900900

901901
// Test first De Morgan's law: NOT(A AND B) = (NOT A) OR (NOT B)
902-
if let (Ok(a_and_b), Ok(not_a), Ok(not_b)) = (and(array, mask), invert(array), invert(mask)) {
902+
if let (Ok(a_and_b), Ok(not_a), Ok(not_b)) =
903+
(and_kleene(array, mask), invert(array), invert(mask))
904+
{
903905
let not_a_and_b =
904906
invert(&a_and_b).vortex_expect("invert should succeed in conformance test");
905907
let not_a_or_not_b =
906-
or(&not_a, &not_b).vortex_expect("or should succeed in conformance test");
908+
or_kleene(&not_a, &not_b).vortex_expect("or should succeed in conformance test");
907909

908910
assert_eq!(
909911
not_a_and_b.len(),
@@ -927,10 +929,12 @@ fn test_boolean_demorgan_consistency(array: &dyn Array) {
927929
}
928930

929931
// Test second De Morgan's law: NOT(A OR B) = (NOT A) AND (NOT B)
930-
if let (Ok(a_or_b), Ok(not_a), Ok(not_b)) = (or(array, mask), invert(array), invert(mask)) {
932+
if let (Ok(a_or_b), Ok(not_a), Ok(not_b)) =
933+
(or_kleene(array, mask), invert(array), invert(mask))
934+
{
931935
let not_a_or_b = invert(&a_or_b).vortex_expect("invert should succeed in conformance test");
932936
let not_a_and_not_b =
933-
and(&not_a, &not_b).vortex_expect("and should succeed in conformance test");
937+
and_kleene(&not_a, &not_b).vortex_expect("and should succeed in conformance test");
934938

935939
for i in 0..not_a_or_b.len() {
936940
let left = not_a_or_b

vortex-array/src/compute/list_contains.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ fn constant_list_scalar_contains(
240240
)?
241241
.fill_null(false_scalar.clone())?;
242242
if let Some(acc) = result {
243-
result = Some(compute::or(&acc, &res)?)
243+
result = Some(compute::or_kleene(&acc, &res)?)
244244
} else {
245245
result = Some(res);
246246
}

vortex-array/src/expr/exprs/operators.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@ pub enum Operator {
3535
/// Boolean AND (∧).
3636
And,
3737
/// Boolean OR (∨).
38+
// TODO(joe): rename to KleeneOr
3839
Or,
3940
/// The sum of the arguments.
4041
///
4142
/// Errs at runtime if the sum would overflow or underflow.
43+
// TODO(joe): rename to KleeneAnd
4244
Add,
4345
/// The difference between the arguments.
4446
///

0 commit comments

Comments
 (0)