fix(postprocessing): preserve NULL index values through pivot() (#43547)

[Archita-kale]

@kokhlo I’ve implemented the follow-up fix discussed here, preserving NULL grouping values through the pivot post-processing path. I’ve also added regression tests for flat and MultiIndex pivots. Would appreciate your review!

SUMMARY

Follow-up fix for #43547 to preserve NULL/NaN grouping values through the pandas post-processing pivot() operator.

Problem
In superset/utils/pandas_postprocessing/pivot.py, NULL values in the columns parameter were already filled with NULL_STRING ("<NULL>") before pivot_table(). However, index columns lacked this handling. Because pandas df.pivot_table() drops rows with NaN in index/grouping keys regardless of the dropna= setting, grouping rows with NULL values that survived the aggregate step were silently dropped during pivoting.

Solution

• Mirrored the existing columns fill pattern: filled index columns with NULL_STRING ("<NULL>") prior to calling df.pivot_table().
• Preserved existing drop_missing_columns behavior without modifying dropna=drop_missing_columns.
• Reused the existing NULL_STRING constant without introducing any new constants or modifying PR fix(pandas-postprocessing): preserve null grouping values #43682.
• Added comprehensive unit tests for both flat pivots and columns MultiIndex pivots with NULL index values.

TESTING INSTRUCTIONS

Run the pandas postprocessing unit tests:

pytest tests/unit_tests/pandas_postprocessing/test_pivot.py -v

[bito-code-review]

Code Review Agent Run #044e78

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: 1017a13..1017a13

  • superset/utils/pandas_postprocessing/pivot.py
  • tests/unit_tests/pandas_postprocessing/test_pivot.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[bito-code-review]

The issue is that df[index].fillna(value=NULL_STRING) fails when the index column is of type Categorical, as NULL_STRING is not in the existing categories. To resolve this, you must ensure the category is added before filling.

Here is the corrected implementation for superset/utils/pandas_postprocessing/pivot.py:

    # Fill NULL/NaN values in the index columns with NULL_STRING so that
    # NULL grouping keys survive as a real "<NULL>" row in the pivot output.
    for col in index:
        if df[col].dtype.name == "category" and NULL_STRING not in df[col].cat.categories:
            df[col] = df[col].cat.add_categories([NULL_STRING])
    df[index] = df[index].fillna(value=NULL_STRING)

This approach safely handles categorical data by adding the missing category before performing the fill operation. There are no other comments on this PR to address.

superset/utils/pandas_postprocessing/pivot.py

for col in index:
        if df[col].dtype.name == "category" and NULL_STRING not in df[col].cat.categories:
            df[col] = df[col].cat.add_categories([NULL_STRING])
    df[index] = df[index].fillna(value=NULL_STRING)

[rusackas]

Hmm... the fact that Cypress AND Playwright failed makes me think there might be a real test problem, but I'll restart them now for good measure in case it is just being flaky.

[Copilot]

Pull request overview

This PR updates Superset’s pandas post-processing pivot() operator to preserve NULL/NaN grouping keys on the index axis (preventing pandas pivot_table() from silently dropping those rows), and adds regression coverage to ensure NULL-index groups survive both flat and MultiIndex pivots.

Changes:

• Add a shared helper to fill missing values in pivot dimension columns (categorical/date-aware handling) before calling pivot_table().
• Fill index dimensions with NULL_STRING ("<NULL>") prior to pivoting so NULL index groups are retained.
• Add unit tests covering NULL index preservation across common dtypes and pivot shapes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
superset/utils/pandas_postprocessing/pivot.py	Adds dimension fill helper and applies it to both columns and index prior to pivot_table() to preserve NULL index keys.
tests/unit_tests/pandas_postprocessing/test_pivot.py	Adds regression tests validating NULL index preservation for flat and MultiIndex pivots across multiple dtypes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	2492275
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6abf03f27d55b50008a1cab0
😎 Deploy Preview	https://deploy-preview-43693--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[bito-code-review]

Code Review Agent Run #449c83

Actionable Suggestions - 0

Additional Suggestions - 1

• superset/utils/pandas_postprocessing/pivot.py - 1

  • datetime coerced to string · Line 203-207
    When any NaT exists, `s.astype(str)` converts the entire datetime column to object dtype, so the pivot index becomes object and sorts lexicographically. For timezone-aware datetimes this can differ from chronological order (e.g. '10:00' before '09:00'). Prior code raised TypeError, so this is an improvement, but confirm downstream time-series consumers tolerate an object index.

Review Details

• Files reviewed - 2 · Commit Range: 464adc9..5a3e631

  • superset/utils/pandas_postprocessing/pivot.py
  • tests/unit_tests/pandas_postprocessing/test_pivot.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.93%. Comparing base (abdf2e6) to head (2492275).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #43693       +/-   ##
===========================================
+ Coverage   57.36%   81.93%   +24.56%     
===========================================
  Files        2986     2986               
  Lines      183289   183306       +17     
  Branches    42385    42394        +9     
===========================================
+ Hits       105144   150187    +45043     
+ Misses      77109    30383    -46726     
- Partials     1036     2736     +1700     

Flag	Coverage Δ
hive	36.54% <5.55%> (-0.01%)	⬇️
mysql	55.53% <33.33%> (?)
postgres	55.54% <33.33%> (?)
presto	38.44% <5.55%> (-0.01%)	⬇️
python	85.95% <100.00%> (+47.46%)	⬆️
sqlite	55.26% <33.33%> (?)
unit	78.92% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[bito-code-review]

Code Review Agent Run #4bd6a5

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: 5a3e631..18c88b5

  • superset/utils/pandas_postprocessing/pivot.py
  • tests/unit_tests/pandas_postprocessing/test_pivot.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[rusackas]

@Archita-kale thanks for the fix, and for the thorough tests! One thing before I approve, though.

The datetime branch in _fill_dimension_column worries me. The same pivot() is what the ECharts timeseries charts run with the time column as the index, and when one NaT shows up it now converts the whole index to strings. That drops the DatetimeIndex, so anything downstream that needs one breaks... resample raises "Resample operation requires DatetimeIndex" where master just dropped the NULL row, and the x-axis values come back as strings instead of timestamps. I think the safer move is to leave datetime dimensions alone (keep dropping NaT rows there) and fix NULL preservation for the categorical/string dims this issue is actually about. Happy to re-review once that's in!

[rusackas]

@Archita-kale holler if you need a hand in validating/addressing the comments above. Thanks!

[rusackas]

@Archita-kale this had gone quiet for a couple weeks with @sadpandajoe's last two catches still open, so I went ahead and pushed fixes for both in bf359f7 rather than wait longer: the categorical fill no longer materializes an unobserved <NULL> bucket for dimensions with no missing values, and nullable extension dtypes (Int64, Float64, boolean) get cast to object before filling instead of raising.

All 44 pivot tests plus the full 153-test pandas_postprocessing suite pass, and pre-commit's clean on the changed files.

@sadpandajoe want to give this one final look, given you did all the real work catching these?

[bito-code-review]

Code Review Agent Run #13e699

Actionable Suggestions - 1

• superset/utils/pandas_postprocessing/pivot.py - 1

  • Datetime type loss · Line 216-216

Additional Suggestions - 1

• tests/unit_tests/pandas_postprocessing/test_pivot.py - 1

  • duplicated test logic · Line 885-1188
    These 13 tests repeat the same shape: build a 3-row DataFrame, call `pivot`, assert `NULL_STRING in result.index`, assert `result.loc[NULL_STRING, ...] == 99`. The only deltas are dtype and columns/column_fill_value. A `@pytest.mark.parametrize` over (index_dtype, columns, fill_value, expected_label) would cut ~300 lines and make the dtype matrix explicit. Divergence risk if one variant is edited and others drift.

Review Details

• Files reviewed - 2 · Commit Range: 8b66e34..514deca

  • superset/utils/pandas_postprocessing/pivot.py
  • tests/unit_tests/pandas_postprocessing/test_pivot.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[rusackas]

Pushed 2492275:

• Rebased onto current master (this branch was ~279 commits stale) — the failing checks were a stale pot-drift/docker-cache-version mismatch already fixed on master since this branch's last sync, not an issue with the pivot fix itself.
• Fixed the one open bito-code-review finding: the datetime branch of _fill_dimension_column was casting the whole column to str, which also stringified every non-null Timestamp value, not just the NaT positions. Switched to astype(object) so non-null entries stay real Timestamp objects; added a regression test for it.
• Resolved the two earlier sadpandajoe threads (categorical unobserved-bucket guard, nullable extension dtype cast) — confirmed those fixes are already in the history, just under a new SHA post-rebase.

Will keep an eye on CI.

[rusackas]

CI is fully green now (mergeStateStatus: CLEAN). Ready to merge.

[bito-code-review]

Bito Automatic Review Skipped – PR Already Merged

Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.
No action is needed if you didn't intend to review it. To get a review, you can type /review in a comment and save it