Skip to content

fix: output_bytes metric in hash aggregation#23777

Open
ariel-miculas wants to merge 2 commits into
apache:mainfrom
ariel-miculas:fix-output-bytes-metric-in-hash-agg
Open

fix: output_bytes metric in hash aggregation#23777
ariel-miculas wants to merge 2 commits into
apache:mainfrom
ariel-miculas:fix-output-bytes-metric-in-hash-agg

Conversation

@ariel-miculas

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Summing together the produced sliced RecordBatches in hash aggregation leads to inflated output_bytes metric

What changes are included in this PR?

Are these changes tested?

Yes

Are there any user-facing changes?

If the metrics are user-facing, then yes.

@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates physical-plan Changes to the physical-plan crate labels Jul 21, 2026
@codecov-commenter

codecov-commenter commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.92965% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.71%. Comparing base (eef1017) to head (ab748ff).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...fusion/physical-plan/src/aggregates/hash_stream.rs 81.20% 7 Missing and 21 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23777      +/-   ##
==========================================
- Coverage   80.71%   80.71%   -0.01%     
==========================================
  Files        1089     1089              
  Lines      368748   368928     +180     
  Branches   368748   368928     +180     
==========================================
+ Hits       297633   297771     +138     
- Misses      53372    53390      +18     
- Partials    17743    17767      +24     

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

@Jithendra2608

Copy link
Copy Markdown

Hi @ariel-miculas! I was taking a look at the failing CI checks to see if I could help unblock this.
It looks like the typos job is failing because of a small spelling error in datafusion/physical-expr-common/src/metrics/baseline.rs on line 326 (accross instead of across).

Fixing that one word should turn the CI green. Hope this helps, and great work on the hash aggregation metrics!

@ariel-miculas

Copy link
Copy Markdown
Contributor Author

@2010YOUY01 coud you please take a look?

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

Labels

physical-expr Changes to the physical-expr crates physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect output_bytes metric in hash aggregation

3 participants