Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24698 +/- ##
==========================================
- Coverage 82.45% 82.44% -0.01%
==========================================
Files 1140 1141 +1
Lines 436434 437085 +651
Branches 436434 437085 +651
==========================================
+ Hits 359840 360355 +515
- Misses 54840 54879 +39
- Partials 21754 21851 +97 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
xavlee
force-pushed
the
feat/issue-24438-group-contiguous-property
branch
from
August 26, 2026 19:20
b2ceb1a to
6bd697f
Compare
xavlee
force-pushed
the
feat/issue-24438-group-contiguous-property
branch
5 times, most recently
from
August 28, 2026 19:45
1e7d83a to
72d66b3
Compare
This was referenced Aug 28, 2026
xavlee
force-pushed
the
feat/issue-24438-group-contiguous-property
branch
9 times, most recently
from
September 4, 2026 19:21
fa946cb to
db683f4
Compare
xavlee
force-pushed
the
feat/issue-24438-group-contiguous-property
branch
from
September 4, 2026 19:30
db683f4 to
3e40948
Compare
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
xavlee
force-pushed
the
feat/issue-24438-group-contiguous-property
branch
2 times, most recently
from
September 7, 2026 20:23
4b4cf23 to
4cbcc31
Compare
xavlee
force-pushed
the
feat/issue-24438-group-contiguous-property
branch
from
September 16, 2026 14:25
4cbcc31 to
41b2813
Compare
gene-bordegaray
approved these changes
Sep 22, 2026
gene-bordegaray
left a comment
Contributor
There was a problem hiding this comment.
some nits but overall this is great work, thank you @xavlee
xavlee
force-pushed
the
feat/issue-24438-group-contiguous-property
branch
3 times, most recently
from
September 22, 2026 21:38
f04b475 to
e284bb8
Compare
xavlee
force-pushed
the
feat/issue-24438-group-contiguous-property
branch
from
September 23, 2026 02:04
e284bb8 to
c2fe167
Compare
xavlee
marked this pull request as ready for review
September 23, 2026 19:30
zhuqi-lucas
pushed a commit
to zhuqi-lucas/arrow-datafusion
that referenced
this pull request
Sep 28, 2026
## Which issue does this PR relate to? - Part of apache#24438. ## Rationale for this change A source can concatenate several sorted logical runs into one DataFusion output partition. The resulting stream may be globally unsorted while every distinct `(key, time_bin)` tuple still occupies one contiguous range. This PR records how aggregate planning handles that layout before grouped input properties are available. It provides the behavioral baseline for the remaining PRs in the stack. ## What changes are included in this PR? - Add a single-partition `TestMemoryExec` fixture containing two sorted logical runs whose `(key, time_bin)` order resets at the record-batch boundary. - Aggregate by the complete `(key, time_bin)` tuple and verify the result. - Assert that planning selects `InputOrderMode::Linear`, `EmissionType::Final`, and `SingleHashAggregateStream`. ## Stack 1. [apache#24737 — test: cover unsorted contiguous groups in one partition](apache#24737) ← **this PR** 2. [apache#24697 — refactor: separate aggregate group completion from input ordering](apache#24697) 3. [apache#24698 — feat: add grouped equivalence properties](apache#24698) 4. [apache#24497 — feat: stream aggregates over grouped input](apache#24497) ## Are these changes tested? The new aggregate test executes the single-partition input and snapshot-checks all four grouped sums. ## Are there any user-facing changes? No. ## Review this layer [View only this PR layer](apache@36969e7)
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
Lexicographical ordering is stronger than the guarantee required to recognize completed groups. A group is complete when all rows with the same grouping-key tuple occur in one contiguous run within a partition, even when the distinct tuples occur in an arbitrary order.
Representing this guarantee in expression equivalence analysis lets DataFusion normalize grouping expressions, derive grouping from existing orderings, project grouping through expressions, and invalidate it at operators that change the row sequence or combine partitions.
What changes are included in this PR?
Semantics
A grouping assertion applies to one complete expression tuple. Grouping on
[a, b]means rows with the same pair of values are contiguous. It establishes no grouping assertion for[a]or[b]individually.[a, b]and[b, a]identify the same groups because changing the order of expressions does not change tuple equality.Lexicographical ordering retains its prefix implication. An ordering on
[a, b, c]establishes grouping for[a],[a, b], and[a, b, c], including equivalent permutations of those complete tuples. It does not establish grouping for[b].Grouping assertions are correctness guarantees supplied by execution-plan producers. DataFusion does not inspect the rows to verify them. A projection retains an explicit tuple only when every expression in that tuple maps to the output. Operators that reorder rows or combine partitions clear explicit assertions whose guarantees no longer hold.
SortProperties::Groupedsupports propagation for a single expression. Scalar functions retain it only for strictly order-preserving transformations, because a many-to-one transformation can map separate input runs to the same output value. For example,date_bindoes not deriveGroupedfrom a grouped timestamp expression. A producer can instead declare the complete derived tuple it guarantees, such as[bhandle, date_bin(timestamp)].Public API Changes
SortProperties::Grouped.GroupingEquivalenceClasstype with construction, insertion, lookup, clearing, schema-rewriting, iteration, and display support.EquivalenceProperties::geq_class,normalized_geq_class,add_grouping,add_groupings,clear_groupings, andgrouping_satisfy.FFI_SortProperties::Groupedand its conversions.TestMemoryExec::try_with_grouping_informationfor physical-plan tests and examples.Execution-plan implementations attach grouping assertions through the existing
PlanProperties::equivalence_properties. The aggregate runtime consumes this metadata in #24497; itsGroupCompletionModeremains an internal implementation detail introduced by #24697.Implementation
EquivalenceProperties.ProjectionMapping.Stack
Are these changes tested?
Tests cover tuple semantics, normalization through equivalences and constants, ordering-derived grouping, projection mapping, strictly order-preserving expression propagation, schema replacement, row-reordering and partition-merging boundaries, display output, and FFI round trips.
Are there any user-facing changes?
Yes.
SortPropertiesis an exhaustive public enum, so addingGroupedrequires downstream exhaustive matches to add an arm. Consumers that do not use grouping information can handle it conservatively withUnordered, for example:FFI_SortPropertiesgains the corresponding variant. Separately compiled FFI consumers should rebuild against the matching DataFusion version and handleGroupedwhen matching this enum.GroupingEquivalenceClassand the newEquivalencePropertiesmethods are additive APIs. The migration guidance for the exhaustive enum additions will also be recorded in the version-specific upgrade guide before merge.Review this layer
View only this PR layer