rluvaton opened a new pull request, #27309: URL: https://github.com/apache/datafusion/pull/27309
## Which issue does this PR close? No issue, this is a follow-up to #18329 which did the same thing for `CASE`. ## Rationale for this change When `BinaryExpr` short-circuits, it filters the whole input batch before evaluating the remaining side: - `OR` pre-selection filters the batch down to the rows where the left side is `false` before evaluating the right side. - `AND` conjunction evaluation filters the batch down to the undecided rows before evaluating the remaining conjuncts. In both cases every column of the batch gets copied, even if the expression only reads one or two of them. With wide batches most of the filtering work is wasted. `CASE` already avoids this by projecting the batch to the used columns and rewriting the column indices. ## What changes are included in this PR? - Move the column projection logic out of `case.rs` into a new `ColumnProjection` helper (`expressions/column_projection.rs`), and make `CASE` use it. No behavior change for `CASE`. - `BinaryExpr` lazily derives the projected children (right side for `OR`, all conjuncts for `AND`) on first use and caches them. - `OR` pre-selection projects the batch to the columns used by the right side before filtering. - `AND` conjunction projects the batch to the columns used by the conjuncts on the first filter and evaluates the projected conjuncts from then on. - Same safety rule as `CASE`: if an expression has an unknown leaf that may read columns not exposed through its children, fall back to filtering the full batch. - `Debug` for `BinaryExpr` is now implemented manually so the output stays the same, and the cache is ignored in proto serialization. ## What is the testing strategy for this PR? - Added `test_conjunction_filters_only_used_columns` and `test_or_pre_selection_filters_only_used_columns`, which check both the result and the number of columns each sub-expression sees (`RowCountRecorder` now also records the column count). - Existing `CASE` / `BinaryExpr` unit tests and sqllogictests pass. Benchmarks (`binary_op`, compared to `main`): | benchmark | change | |---|---| | `short_circuit/or/all_false_in_or` | -26% | | `short_circuit/or/one_false_middle_right` | -31% | | `short_circuit/or/one_false_*` (others) | -4% to -8% | | `conjunction/*_regex_w32` | -10% to -11% | | `conjunction/k4_p0.5_nulls_regex_w32` | -6% to -13% | | `conjunction/k8_p0.1_then0.9*` | -3% to -10% | | `conjunction/k8_p0.9_w32` | -14% to -17% | Cases where no filtering happens (cheap conjuncts) should be unchanged. Most were within noise, but `conjunction/k8_p0.7_w32` and `conjunction/k8_p0.7_plus_w32/right_deep` showed +4% to +10% on a rerun. These don't go through the changed code, so I believe it's noise, but worth a second run. ## Are there any user-facing changes? No, only performance. -- 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]
