freakyzoidberg opened a new pull request, #27303:
URL: https://github.com/apache/datafusion/pull/27303

   ## Which issue does this PR close?
   
   - Closes #27301.
   
   ## Rationale for this change
   
   Grouped `first_value` / `last_value` with `ORDER BY` keep each group's 
ordering as a `Vec<ScalarValue>`. Every batch winner is converted to 
`ScalarValue`s and compared with `compare_rows`, and every batch scans a 
scoreboard sized to all groups. With many groups, this dominates the query.
   
   ## What changes are included in this PR?
   
   - Add `FirstLastPrimitiveKeyGroupsAccumulator<K>`, used when there is 
exactly one ORDER BY expression of primitive type. It is picked with 
`downcast_primitive!`, like `GroupValuesPrimitive` for GROUP BY keys. It keeps 
the orderings in a `Vec<K::Native>` plus a null bitmap and compares them 
directly, in one pass per batch.
   - Add `ValueState::update_rows`, so the value state is updated once per 
batch.
   - The partial state (`[value, ordering, is_set]`) and the rows picked, ties 
included, are the same as with the existing accumulator, which still handles 
every other ordering.
   
   ## What is the testing strategy for this PR?
   
   - `first_last_primitive_key.slt` compares `ORDER BY k` with `ORDER BY k, u`. 
`u` is unique, so the existing accumulator is the oracle. It covers:
     - key types with their own comparison: Int8, UInt64, Float16 / Float64 
with NaN and -0.0, Decimal128 / 256, Timestamp, Interval;
     - every ASC / DESC × NULLS combination;
     - IGNORE NULLS, FILTER, and skip partial aggregation.
   - Unit tests for the routing and for ties.
   - Existing single-key tests (`first_last_ordered.slt`, `group_by.slt`, 
`aggregate.slt`) now run on the new path unchanged.
   
   ## Benchmarks
   
   `cargo bench` on an M4 Max, `main` vs this PR:
   
   | bench | main | this PR |
   |---|---|---|
   | `first_last` `first_value update_bench nulls=0%, filter=false` | 22.3 ms | 
8.0 ms |
   | `first_last` `last_value update_bench nulls=90%, filter=true` | 39.3 ms | 
5.1 ms |
   | `first_last` `first_value merge_bench nulls=0%, filter=false` | 28.4 ms | 
8.0 ms |
   | `first_last` `first_value evaluate_bench nulls=0%, filter=false, all` | 
10.1 µs | 0.28 µs |
   | `first_last` `last_value update_bench struct(i64,utf8,f64) nulls=0%` | 
25.8 ms | 10.4 ms |
   | `aggregate_query_sql` `first_last_one_column` (10 groups) | 697 µs | 660 
µs |
   | `aggregate_query_sql` `first_last_many_columns` (unchanged path) | 822 µs 
| 829 µs |
   
   
   ## Are there any user-facing changes?
   
   No.
   
   ## AI usage
   
   Written with the help of AI tools
   I've reviewed and understand the code.


-- 
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]

Reply via email to