jayzhan211 commented on code in PR #25771:
URL: https://github.com/apache/datafusion/pull/25771#discussion_r4166496406
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -1426,6 +1397,135 @@ fn pre_selection_scatter(
Ok(ColumnarValue::Array(Arc::new(boolean_result)))
}
+/// Selects undecided rows when some are false, at most
+/// [`PRE_SELECTION_THRESHOLD`] of the original batch remain, and evaluating
+/// `conjunct` on all rows is not cheap and infallible.
+fn rows_to_filter_before(
+ conjunct: &Arc<dyn PhysicalExpr>,
+ result: &ColumnarValue,
+ schema: &Schema,
+ num_rows: usize,
+) -> Option<BooleanArray> {
+ let ColumnarValue::Array(array) = result else {
+ return None;
+ };
+ let undecided = not_false(array.as_boolean());
+ let undecided_count = undecided.true_count();
+ // Classify last: it walks the whole conjunct.
Review Comment:
I could reproduce the `selectivity_q21` loss you disclosed. On a 3-column
Int32 batch of 8192 rows, a clustered `a < t` prefix with ~1% survivors
followed by cheap `x < 70` conjuncts is 1.14x slower with one cheap conjunct
(2.03 → 2.32 µs) and 1.24x with two (2.81 → 3.48 µs). At 5–20% survivors, or
with randomly placed survivors, the PR is 0.26–0.99x, so a lower plain
threshold would hurt the random case.
I'm fine accepting this as the trade-off. Can you file a follow-up to
explore filtering before a cheap conjunct when the mask is a few long runs (a
small slice count from `SlicesIterator`)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
