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

   ## Which issue does this PR close?
   
   - Closes #23715.
   
   ## Rationale for this change
   
   Queries using `array_agg(DISTINCT col)` were significantly slower than 
expected.
   
   Profiling revealed that `DistinctArrayAggAccumulator::update_batch` was 
allocating a
   heap-owned `String` on every single input row — even for rows whose value 
was already
   present in the accumulator. For a typical low-cardinality workload (e.g. a 
column of ~25
   database names across thousands of rows), this meant paying the full 
allocation cost for
   every duplicate, which dominated the runtime.
   
   ## What changes are included in this PR?
   
   This PR applies the same deduplication strategy already used by 
`AggregateExec` for
   `GROUP BY`: duplicate rows now cost only a hash probe with no heap 
allocation, and new
   distinct values are appended to a single shared buffer rather than allocated 
individually.
   The fix applies to all column types, not just strings, and `retract_batch` 
support
   (required for sliding window frames such as `ROWS BETWEEN N PRECEDING AND 
CURRENT ROW`)
   is fully preserved.
   
   ## Are these changes tested?
   
   Four unit tests were added to `DistinctArrayAggAccumulator` — one each for 
`Utf8`,
   `Int64`, `Float64`, and `Date32` — to pin the deduplication contract across 
the most
   common column types and serve as a regression guard for future changes. The 
existing
   sliding window sqllogictest suite (`array_agg_sliding_window.slt`) covers 
`retract_batch`
   correctness end-to-end and passes unchanged.
   
   Two `update_batch` micro-benchmarks were added to measure the before/after 
on realistic
   data: one with low cardinality (~25 distinct database names in 8 192 rows, 
modelling the
   common production case) and one with high cardinality (~7 800 distinct 
values, modelling
   the worst case where almost every row is new). Results on an 8 192-row batch:
   
   | Benchmark | Before | After | Speedup |
   |---|---|---|---|
   | Low cardinality (~25 distinct DB names) | 648.6 µs | 189.4 µs | **3.42×** |
   | High cardinality (~7 800 distinct values) | 1078.3 µs | 269.4 µs | 
**4.00×** |
   
   
   ## Are there any user-facing changes?
   
   No user-facing changes
   
   No breaking changes to public APIs


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