zhuqi-lucas opened a new issue, #24771:
URL: https://github.com/apache/datafusion/issues/24771
### Is your feature request related to a problem or challenge?
`OptimizeAggregateOrder` (`physical-optimizer/src/update_aggr_exprs.rs`)
detects when the input ordering already satisfies a `first_value`/`last_value`
`ORDER BY` requirement and calls `with_beneficial_ordering(true)`, which sets
`is_input_pre_ordered` on the UDAF.
The per-group `Accumulator` path honors it:
`FirstValueAccumulator`/`LastValueAccumulator` skip all comparisons and just
take the first/last (non-null) row
(`functions-aggregate/src/first_last.rs:906`).
The `GroupsAccumulator` path does not: `create_groups_accumulator` never
threads the flag through, and `FirstLastGroupsAccumulator` has no pre-ordered
branch at all. So as soon as the query has a `GROUP BY`, the full machinery
runs on every batch even when the input is perfectly sorted:
- build a `LexicographicalComparator` over the ordering columns,
- run the per-row tournament in `get_filtered_extreme_of_each_group`,
- materialize the winning row's ordering key into `ScalarValue`s
(`extract_row_at_idx_to_buf`),
- cross-batch `compare_rows` against the stored per-group orderings.
None of that is needed when the input is pre-ordered: for `last_value` the
last seen row per group always wins, so `update_batch` degenerates to an
unconditional overwrite per `(group, row)` — no comparator, no `orderings`
boxing.
**Motivating workload**: a materialized aggregation over an options NBBO
table (~1.5B rows/day, ~1.8M groups/day) whose file sort order is exactly
`(ticker, block_timestamp, sequence_number)`:
```sql
SELECT date, ticker,
LAST_VALUE(block_timestamp ORDER BY block_timestamp, sequence_number),
LAST_VALUE(bid_price ORDER BY block_timestamp, sequence_number),
LAST_VALUE(ask_price ORDER BY block_timestamp, sequence_number)
FROM nbbo_quotes
GROUP BY date, ticker
```
The group keys are a prefix of the sort order, and within each group the
rows are already sorted by the aggregate's requirement. A CPU profile of this
query shows ~20% of on-CPU time inside `FirstLastGroupsAccumulator` (dominated
by `get_filtered_extreme_of_each_group`), all of it avoidable comparisons.
### Describe the solution you'd like
Mirror the existing single-group fast path in the grouped one:
1. Pass `is_input_pre_ordered` through `create_groups_accumulator` into
`FirstLastGroupsAccumulator`.
2. When set, replace the tournament in `update_batch` with a single pass
over `group_indices` that records the first (for `first_value`) or last (for
`last_value`) qualifying row index per group in the batch, then updates state
unconditionally (later batches always win for `last_value`; for `first_value`,
only unset groups are written). The `orderings` vector and comparator are not
needed in this mode.
The optimizer-side detection already exists; this is only about consuming
the flag symmetrically.
### Describe alternatives you've considered
- Leaving it to the planner to rewrite `last_value` into something cheaper:
there is nothing cheaper to rewrite into today; the accumulator is the right
place.
- #23682 (coalescing same-`ORDER BY` first/last into one struct accumulator)
helps the unsorted case and is complementary; it does not remove the per-row
comparisons when the input is already sorted.
I can work on this.
--
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]
