Dandandan opened a new pull request, #23205: URL: https://github.com/apache/datafusion/pull/23205
## Which issue does this PR close? <!-- No issue filed; self-contained window perf change. Happy to open one if preferred. --> - Closes #. ## Rationale for this change On the non-streaming `WindowAggExec` path, frame-using window functions go through `StandardWindowExpr::evaluate`, which calls `PartitionEvaluator::evaluate` **once per row** — boxing a `ScalarValue` per row and reassembling with `ScalarValue::iter_to_array`. For `first_value` / `last_value` / `nth_value` the per-row result is just a positional gather from the input column, so the `ScalarValue` boxing is pure overhead that a single `take` kernel can replace. ## What changes are included in this PR? - Two **opt-in** `PartitionEvaluator` methods: `supports_evaluate_all_with_frame_ranges` (default `false`) and `evaluate_all_with_frame_ranges` (default `not_impl_err!`). Existing evaluators are unaffected. - `StandardWindowExpr::evaluate` uses them when the evaluator opts in (computing the frame ranges up front, then one vectorized call), and otherwise keeps the existing per-row loop verbatim. - `NthValueEvaluator` implements them for the common `IGNORE NULLS == false` case: it builds a single `UInt32` gather index from the per-row frame ranges (null for empty frames / out-of-range `Nth`) and produces the whole output column with one `arrow::compute::take`. The `IGNORE NULLS` path (which walks the null bitmap per row) deliberately stays on the scalar path. Scope note: this only affects the non-streaming `WindowAggExec` path (e.g. frames ending in `UNBOUNDED FOLLOWING`). The streaming `BoundedWindowAggExec` path is untouched and could be addressed separately. ## Are these changes tested? Yes: - A new differential unit test asserts the vectorized result matches the per-row `evaluate` output element-for-element, covering NULLs, empty frames, and `first`/`last`/`nth(2)`/`nth(-1)`. - The full `window` sqllogictest suite passes (all 6 files). ## Are there any user-facing changes? No behavior change. This **adds two defaulted methods to the public `PartitionEvaluator` trait** (a backwards-compatible API addition) — may warrant the `api change` label. -- 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]
