Skip to content

fix: split BoundedWindowAggExec output into batch_size batches - #26974

Merged
comphead merged 3 commits into
apache:mainfrom
comphead:fix-26959-bounded-window-batch-size
Nov 3, 2026
Merged

comphead merged 3 commits into
apache:mainfrom
comphead:fix-26959-bounded-window-batch-size

Conversation

@comphead

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

BoundedWindowAggExec emits every row that becomes final in one batch, so a long RANGE peer group, ROWS ... N FOLLOWING or lead(x, N) can produce batches far larger than datafusion.execution.batch_size. The issue has a reproducer.

What changes are included in this PR?

execute wraps the stream in BatchSplitStream and ObservedStream, as UnnestExec does since #24384. Larger batches go out as zero-copy slices of at most batch_size rows, and output_batches counts the slices.

What is the testing strategy for this PR?

A new window.slt case runs EXPLAIN ANALYZE with batch_size = 2 over two 5-row peer groups, one finishing mid-stream and one at the end of the input. BoundedWindowAggExec reports output_batches=6, against 2 on main. bounded_window_linear_quiet_partition_resume_standard now uses the default batch size, so it still checks when rows become final. The bounded_window benchmark is within 3% of main on every case, in both directions (best of six alternating runs).

Are there any user-facing changes?

Only batch boundaries change, and BoundedWindowAggExec gains a batches_split metric. Known limitations:

  • Small batches are not merged. With repeated ORDER BY keys, an emission slightly over batch_size becomes a full batch plus a short one. sum(v) OVER (ORDER BY k) over 1M rows with 3-row peer groups goes from 124 to 205 output batches.
  • The slices share their parent's buffers. Operators that reserve get_array_memory_size() per input batch, such as NestedLoopJoinExec and CrossJoinExec, charge each slice for the whole parent, as they already do for aggregate output. A nested loop join building on a 65,536-row peer group at batch_size = 1024 reserves 96 MB instead of 1.5 MB. HashJoinExec counts shared buffers once since fix: count shared buffers once in hash join build-side memory accounting #22862.

`BoundedWindowAggExec` emitted every row that became final in one batch,
so frames that need later rows could exceed `batch_size`. Wrap its stream
in `BatchSplitStream` and `ObservedStream`, as `UnnestExec` does, so the
output is sliced into batches of at most `batch_size` rows and
`output_batches` counts the slices.

Closes apache#26959.
@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Oct 31, 2026

@2010YOUY01 2010YOUY01 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@2010YOUY01
2010YOUY01 added this pull request to the merge queue Nov 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 3, 2026
Phoenix500526 pushed a commit to Phoenix500526/datafusion that referenced this pull request Nov 3, 2026
…sh_collisions (apache#27034)

## Which issue does this PR close?

- Closes apache#27033.

## Rationale for this change

`completed_groups_release_first_value_bitmap` (added in apache#26782) takes
about 10 minutes in the `cargo test hash collisions (amd64)` merge-queue
job. It aggregates 131,072 distinct keys in one batch. Under
`force_hash_collisions`, every key has the hash 0, so the aggregate is
quadratic. The test checks that `FIRST_VALUE(v ORDER BY v)` releases its
per-batch bitmap, which does not depend on hashing.

The long job gives spot runner shutdowns more time to hit it. Evidence
from merge-queue runs:

- In the passing rerun for apache#26926
([job](https://github.com/apache/datafusion/actions/runs/41571721394/job/125710730373)),
the physical-plan unit test binary took 609.89s. All other tests in it
finished by 03:40:31. This test ran from about 03:40:07 to 03:50:17.
- The median job time went from 6.2 minutes (230 jobs, Oct 21 to Oct 30)
to 12.0 minutes (70 jobs after apache#26782 merged).
- All 5 failed attempts of the job since then were runner shutdowns
while this test was running: apache#26929
([job](https://github.com/apache/datafusion/actions/runs/41114163010/job/124263887540)),
apache#26898
([job](https://github.com/apache/datafusion/actions/runs/41115099549/job/124267135386)),
apache#26883
([job](https://github.com/apache/datafusion/actions/runs/41120437208/job/124285496814)),
apache#26974
([job](https://github.com/apache/datafusion/actions/runs/41559552415/job/125666576696)),
apache#26926
([job](https://github.com/apache/datafusion/actions/runs/41571721394/job/125705896210)).

## What changes are included in this PR?

Skip the test under `force_hash_collisions`, as apache#26594 did for another
test. The imports that only this test uses move into its body, so they
are not unused under the feature.

## What is the testing strategy for this PR?

Test config change only. The regular `cargo test` job still runs the
test. Locally (`--profile ci`), the test takes 0.92s by default and 309s
with `force_hash_collisions`. With the feature, it is now skipped, and
Clippy passes with and without the feature.

## Are there any user-facing changes?

No.
@comphead
comphead enabled auto-merge November 3, 2026 16:05
@comphead
comphead added this pull request to the merge queue Nov 3, 2026
Merged via the queue into apache:main with commit 7a3cab8 Nov 3, 2026
43 of 44 checks passed
@comphead
comphead deleted the fix-26959-bounded-window-batch-size branch November 3, 2026 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt) v57.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BoundedWindowAggExec can emit batches larger than batch_size

2 participants