Skip to content

fix(rust): Handle empty lists in list.agg with scalar aggregations#26424

Open
Zakir032002 wants to merge 3 commits intopola-rs:mainfrom
Zakir032002:fix-list-agg-empty-panic
Open

fix(rust): Handle empty lists in list.agg with scalar aggregations#26424
Zakir032002 wants to merge 3 commits intopola-rs:mainfrom
Zakir032002:fix-list-agg-empty-panic

Conversation

@Zakir032002
Copy link

Fixes #26237

When list.agg() is used with scalar aggregations (such as first()) on columns
containing a mix of nulls and empty lists, Polars may panic with an assertion
failure in deposit().

This occurs because scalar aggregations on empty lists can produce no output,
while deposit() expects one value per valid group based on the validity bitmap,
leading to a length mismatch.

This PR pads the aggregation result with null values when necessary so the number
of values matches the number of valid groups before calling deposit().

The fix is applied to functions:

  • evaluate_on_list_chunked
  • evaluate_on_array_chunked

AI disclosure

  • I used AI assistance to help reason about the root cause .
  • I have personally reviewed all code changes and confirm they are correct,
    relevant, and consistent with the existing codebase.

Fixes pola-rs#26237

When list.agg() is called with mixed nulls and empty lists,
empty groups may produce no values. This fix pads the result
with nulls to match expected length before deposit().
@github-actions github-actions bot added fix Bug fix rust Related to Rust Polars labels Feb 4, 2026
@Zakir032002 Zakir032002 marked this pull request as ready for review February 4, 2026 18:04
@codecov
Copy link

codecov bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.01%. Comparing base (c1c140f) to head (1dacac6).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
crates/polars-expr/src/expressions/eval.rs 42.85% 8 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #26424   +/-   ##
=======================================
  Coverage   81.01%   81.01%           
=======================================
  Files        1782     1782           
  Lines      243119   243130   +11     
  Branches     3078     3078           
=======================================
+ Hits       196954   196972   +18     
+ Misses      45362    45355    -7     
  Partials      803      803           

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

I don't think this is the right solution, to me the problem should be fixed elsewhere. I don't see why 'some aggregations may produce fewer values' should be acceptable, and should be hidden by just extending with nulls.

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

Labels

fix Bug fix rust Related to Rust Polars

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Panic for list.agg with mixed nulls and empty lists

2 participants

Comments