Skip to content

perf: use columnar state for ordered ARRAY_AGG and STRING_AGG - #24392

Open
lyne7-sc wants to merge 6 commits into
apache:mainfrom
lyne7-sc:perf/columnar-ordered-array-agg
Open

perf: use columnar state for ordered ARRAY_AGG and STRING_AGG#24392
lyne7-sc wants to merge 6 commits into
apache:mainfrom
lyne7-sc:perf/columnar-ordered-array-agg

Conversation

@lyne7-sc

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Ordered ARRAY_AGG and STRING_AGG currently have high memory and sorting overhead for large inputs.

This is relevant to #20788, where removing ORDER BY from several ARRAY_AGG expressions avoids the reported memory explosion. This PR improves the ordered ARRAY_AGG and STRING_AGG accumulators.

What changes are included in this PR?

This PR changes the ordered aggregate accumulator to:

  • Retain payload values as Arrow arrays.
  • Store compact references to payload rows.
  • Encode ordering keys with RowConverter.
  • Reuse already sorted input and partial-state runs.
  • Sort remaining entries and merge the sorted runs.
  • Materialize the result with Arrow interleave or a zero-copy slice.

The accumulator is shared by ordered ARRAY_AGG and STRING_AGG.

Are these changes tested?

Yes. Unit and SQL logic tests cover ordering, partial-state merging, NULL handling, and type preservation.

Are there any user-facing changes?

No. This is an internal performance optimization

Benchmarks

Benchmark coverage is added in #24391.

Benchmark Main runtime Optimized runtime Runtime improvement Main peak memory Optimized peak memory Memory reduction
Ordered ARRAY_AGG 897.8 ms 85.4 ms 10.5x 404.9 MB 165.1 MB 59.2%
Two ordered ARRAY_AGG expressions 2048.2 ms 150.8 ms 13.6x 795.3 MB 247.8 MB 68.8%
Ordered STRING_AGG 895.1 ms 96.3 ms 9.3x 414.6 MB 158.0 MB 61.9%

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Aug 15, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.40486% with 116 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.19%. Comparing base (bbb552f) to head (bc03e0c).
⚠️ Report is 37 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/functions-aggregate/src/array_agg.rs 83.40% 30 Missing and 86 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24392      +/-   ##
==========================================
+ Coverage   81.14%   81.19%   +0.04%     
==========================================
  Files        1110     1110              
  Lines      386137   389248    +3111     
  Branches   386137   389248    +3111     
==========================================
+ Hits       313347   316047    +2700     
- Misses      54324    54544     +220     
- Partials    18466    18657     +191     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants