Skip to content

fix: Propagate null in min_by / max_by for all-null by groups#26919

Merged
orlp merged 3 commits intopola-rs:mainfrom
gautamvarmadatla:fix/min-max-by-null-group
Mar 16, 2026
Merged

fix: Propagate null in min_by / max_by for all-null by groups#26919
orlp merged 3 commits intopola-rs:mainfrom
gautamvarmadatla:fix/min-max-by-null-group

Conversation

@gautamvarmadatla
Copy link
Copy Markdown
Contributor

Fixes : #26918

Previously, into_no_null_iter() was used on the nullable index result from agg_arg_min/agg_arg_max. That iterator reads raw values from data_views() without checking the validity bitmap, so null entries were treated as their underlying raw buffer value. In this case that became 0, which caused all-null groups to gather the group’s first element.

I fixed this by switching to iter() so the indices stay as Option<IdxSize> through the mapping step, and by using take_unchecked(&IdxCa), which propagates null indices to null outputs.

@github-actions github-actions Bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Mar 14, 2026
@gautamvarmadatla gautamvarmadatla force-pushed the fix/min-max-by-null-group branch from be914aa to fb8e650 Compare March 14, 2026 17:49
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.74%. Comparing base (3ee3f1b) to head (498c92f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #26919   +/-   ##
=======================================
  Coverage   81.73%   81.74%           
=======================================
  Files        1806     1806           
  Lines      248701   248703    +2     
  Branches     3133     3133           
=======================================
+ Hits       203287   203298   +11     
+ Misses      44609    44600    -9     
  Partials      805      805           

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown
Member

@orlp orlp left a comment

Choose a reason for hiding this comment

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

Thanks!

@orlp orlp merged commit 2b825ad into pola-rs:main Mar 16, 2026
29 checks passed
davanstrien pushed a commit to davanstrien/polars that referenced this pull request Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fix python Related to Python Polars rust Related to Rust Polars

Projects

None yet

Development

Successfully merging this pull request may close these issues.

min_by / max_by returns group’s first element instead of null when all by-values are null

2 participants