Skip to content

Commit 1bc4fcb

Browse files
perf: avoid allocating before the first conjunct in AND evaluation
1 parent 7f25fb0 commit 1bc4fcb

1 file changed

Lines changed: 25 additions & 10 deletions

File tree

  • datafusion/physical-expr/src/expressions

‎datafusion/physical-expr/src/expressions/binary.rs‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ use crate::PhysicalExpr;
2121
use crate::expressions::SqlSimilarToPattern;
2222
use crate::expressions::translate_scalar;
2323
use crate::intervals::cp_solver::{propagate_arithmetic, propagate_comparison};
24-
use crate::utils::split_conjunction;
2524
use std::borrow::Cow;
2625
use std::cmp::Ordering;
2726
use std::hash::Hash;
@@ -1213,22 +1212,20 @@ impl BinaryExpr {
12131212
/// filters before conjuncts that may be costly or fail, so they never see
12141213
/// rows nested evaluation would skip. `NULL` rows stay for a later `false`.
12151214
fn evaluate_conjunction(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
1216-
let mut conjuncts = split_conjunction(&self.left);
1217-
conjuncts.extend(split_conjunction(&self.right));
1218-
12191215
// Keep each filter mask for the final scatter.
12201216
let mut selections = vec![];
12211217
let mut input = Cow::Borrowed(batch);
12221218
let mut result = ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)));
12231219

1224-
for conjunct in conjuncts {
1220+
// Returns whether to go on to the next conjunct.
1221+
let mut evaluate = |conjunct: &Arc<dyn PhysicalExpr>| -> Result<bool> {
12251222
if let Some(undecided) = rows_to_filter_before(
12261223
conjunct,
12271224
&result,
12281225
batch.schema_ref(),
12291226
batch.num_rows(),
12301227
) {
1231-
let array = result.into_array(input.num_rows())?;
1228+
let array = result.to_array(input.num_rows())?;
12321229
// Every kept row is true when there are no NULLs.
12331230
result = if array.null_count() == 0 {
12341231
ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))
@@ -1240,10 +1237,13 @@ impl BinaryExpr {
12401237
}
12411238

12421239
let value = conjunct.evaluate(&input)?;
1243-
result = and_kleene_columnar(result, value, input.num_rows())?;
1244-
if is_all_false(&result) {
1245-
break;
1246-
}
1240+
let so_far =
1241+
std::mem::replace(&mut result, ColumnarValue::Scalar(ScalarValue::Null));
1242+
result = and_kleene_columnar(so_far, value, input.num_rows())?;
1243+
Ok(!is_all_false(&result))
1244+
};
1245+
if for_each_conjunct(&self.left, &mut evaluate)? {
1246+
for_each_conjunct(&self.right, &mut evaluate)?;
12471247
}
12481248

12491249
selections
@@ -1461,6 +1461,21 @@ fn is_cheap_and_infallible(expr: &Arc<dyn PhysicalExpr>, schema: &Schema) -> boo
14611461
.all(|child| is_cheap_and_infallible(child, schema))
14621462
}
14631463

1464+
/// Calls `f` on each conjunct of the `AND` chain under `expr`, left to right,
1465+
/// until it returns `false`. Walks the tree in place, so it doesn't allocate.
1466+
fn for_each_conjunct(
1467+
expr: &Arc<dyn PhysicalExpr>,
1468+
f: &mut impl FnMut(&Arc<dyn PhysicalExpr>) -> Result<bool>,
1469+
) -> Result<bool> {
1470+
match expr.downcast_ref::<BinaryExpr>() {
1471+
Some(binary) if binary.op == Operator::And => {
1472+
Ok(for_each_conjunct(&binary.left, f)?
1473+
&& for_each_conjunct(&binary.right, f)?)
1474+
}
1475+
_ => f(expr),
1476+
}
1477+
}
1478+
14641479
/// Whether every row is already `false`.
14651480
fn is_all_false(value: &ColumnarValue) -> bool {
14661481
match value {

0 commit comments

Comments
 (0)