Skip to content

test: cover unsorted contiguous groups in one partition - #24737

Draft
xavlee wants to merge 1 commit into
apache:mainfrom
xavlee:test/issue-24438-group-contiguous-single-partition
Draft

test: cover unsorted contiguous groups in one partition#24737
xavlee wants to merge 1 commit into
apache:mainfrom
xavlee:test/issue-24438-group-contiguous-single-partition

Conversation

@xavlee

@xavlee xavlee commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR relate to?

Rationale for this change

A source can concatenate sorted logical runs into one DataFusion output partition. The resulting rows can be globally unsorted while every distinct (key, time_bin) grouping tuple still occupies one contiguous range. This fixture records how aggregate planning handles that layout today.

What changes are included in this PR?

  • Add a single-partition TestMemoryExec fixture containing two sorted logical runs whose (key, time_bin) order resets between runs.
  • Group by the complete (key, time_bin) tuple.
  • Assert InputOrderMode::Linear, EmissionType::Final, and the ordinary single-hash stream.

Stack

  1. #24737 — test: cover unsorted contiguous groups in one partitionthis PR
  2. #24697 — refactor: separate aggregate group completion from input ordering
  3. #24698 — feat: add narrow group-contiguous source property
  4. #24497 — feat: stream exact group-contiguous aggregates

Are these changes tested?

Yes, this PR itself only adds a test that serves to capture behavior today.

Are there any user-facing changes?

No.

Review this layer

View only this PR layer

@codecov-commenter

codecov-commenter commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.30769% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.58%. Comparing base (ee59f62) to head (5dfc10b).
⚠️ Report is 35 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/physical-plan/src/aggregates/mod.rs 72.30% 3 Missing and 15 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24737      +/-   ##
==========================================
+ Coverage   81.42%   81.58%   +0.15%     
==========================================
  Files        1121     1123       +2     
  Lines      402142   406668    +4526     
  Branches   402142   406668    +4526     
==========================================
+ Hits       327460   331760    +4300     
+ Misses      55484    55457      -27     
- Partials    19198    19451     +253     

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

@xavlee
xavlee force-pushed the test/issue-24438-group-contiguous-single-partition branch from 060dd38 to 3b6d7ab Compare August 27, 2026 20:54

@gene-bordegaray gene-bordegaray left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

approved with some suggstions for displaying expected behavior

Comment thread datafusion/physical-plan/src/aggregates/mod.rs
Comment thread datafusion/physical-plan/src/aggregates/mod.rs Outdated
@xavlee
xavlee force-pushed the test/issue-24438-group-contiguous-single-partition branch from 3b6d7ab to 5dfc10b Compare August 31, 2026 17:50

@gene-bordegaray gene-bordegaray left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this guy looking good now, thank you @xavlee 💃

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants