neilconway opened a new pull request, #24033: URL: https://github.com/apache/datafusion/pull/24033
## Which issue does this PR close? - Related to #23982 ## Rationale for this change In Linear mode, rows arrive ordered on a prefix of the window's ORDER BY expressions, so each new row bounds every row that will arrive in the future, even for other partitions. We exploit this by using the last row to arrive in a batch to close window frames for all live partitions, not just the partition to which that row belongs. The most-recent-row-in-the-batch is conceptually per-batch state, but it was previously implemented as per-partition state: - `update_partition_batch` copied the last row of each incoming batch into every live partition's `PartitionBatchState`. - Each copy is ~(40 + 16 * n_cols) bytes (`Option<RecordBatch>` plus a `Vec` of `ArrayRefs`); for example, withn 100k live partitions over 10 columns, that is ~20MB of duplicate state. - Every partition visit re-evaluated the row's ORDER BY expressions. Instead, just store the row once. Rather than copying the row into every partition, we pass the row to window expression evaluation. This removes `most_recent_row` and its setter from `PartitionBatchState`, which is a breaking API change for datafusion-expr. We can also arrange to evaluate the ORDER BY once per batch instead of once per partition. This is a modest performance improvement and memory savings, but also a conceptual cleanup/refactor. Benchmarks: (using #24032) - linear 100 partitions: 44.4 ms -> 44.5 ms (within noise) - linear 10000 partitions: 203.3 ms -> 199.7 ms (-1.7%) - linear sparse 32768 partitions: 236.9 ms -> 224.5 ms (-5.2%) - linear rows 10000 partitions: 174.2 ms -> 169.4 ms (-2.4%) - linear multi 10000 partitions: 304.6 ms -> 295.7 ms (-3.0%) - sorted 10000 partitions: 34.2 ms -> 34.4 ms (within noise) ## What changes are included in this PR? * Add `WindowEvalContext` parameter to `aggregate_evaluate_stateful` * Remove `PartitionBatchState::most_recent_row` and its setter; instead pass row via `WindowEvalContext` ## Are these changes tested? Yes, covered by existing tests. ## Are there any user-facing changes? Yes, API change to `datafusion-expr`. -- 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]
