refactor: separate aggregate group completion from input ordering - #24697
Draft
xavlee wants to merge 2 commits into
Draft
refactor: separate aggregate group completion from input ordering#24697xavlee wants to merge 2 commits into
xavlee wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24697 +/- ##
==========================================
- Coverage 81.42% 81.42% -0.01%
==========================================
Files 1121 1121
Lines 402142 402197 +55
Branches 402142 402197 +55
==========================================
+ Hits 327460 327501 +41
- Misses 55484 55490 +6
- Partials 19198 19206 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
xavlee
force-pushed
the
refactor/aggregate-group-completion-mode
branch
3 times, most recently
from
August 27, 2026 19:17
72faa6e to
f58e128
Compare
xavlee
force-pushed
the
refactor/aggregate-group-completion-mode
branch
from
August 27, 2026 20:54
f58e128 to
372429f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR relate to?
Rationale for this change
Aggregate execution currently derives both input-order metadata and the runtime mechanism for recognizing completed groups from
InputOrderMode.Completing a group incrementally is contingent on knowing that "after processing a group, we will never see this group again in this stream". As such, input ordering covers only a subset of valid group completion cases. A sorted input guarantees that after processing a group, we will never see it again; however, the general case proves that so long as all rows in a groups are contiguous, the group completion property is satisfied regardless of the input ordering.
Consider the following:
This PR introduces a new GroupCompletionMode that decouples the input ordering from group completion semantics. Currently, we directly map existing InputOrderMode to this property. In a follow up PR, we will read group contiguity expressions to evaluate this property.
What changes are included in this PR?
GroupCompletionMode::{None, Partial, Full}variants.GroupCompletionModefromInputOrderModeduringAggregateExecconstruction.GroupCompletionModethrough ordered aggregate tables, streams, and spill replay paths.InputOrderModefor required ordering, output ordering, andmaintains_input_ordermetadata.GroupOrdering::try_new(&InputOrderMode)as the public compatibility entry point.The ordering-derived conversion is:
Stack
Are these changes tested?
The characterization from #24737 remains
Linear,None,Final, and single-hash after the refactor.Are there any user-facing changes?
No.
GroupCompletionModeis crate-private, and the publicGroupOrderingconstructor is unchanged.Review this layer
View only this PR layer