feat(pivot-table): reintroduce show-values-as-fraction display option - #42761
feat(pivot-table): reintroduce show-values-as-fraction display option#42761rusackas wants to merge 4 commits into
Conversation
Issue #41463 (SIP-216, merged via #41184) correctly moved pivot table totals/subtotals to DB-computed rollup queries, fixing non-additive metrics (ratios, COUNT_DISTINCT, AVG, etc.). As part of that, the Pivot Table's old "Aggregation function" control was removed entirely, taking its "Sum as Fraction of Total/Rows/Columns" options down with it -- collateral damage, not something the SIP identified as broken. Those variants only ever wrapped Sum/Count (both additive), so they were already correct before #41184 and remain a distinct, wanted feature independent of the non-additive-totals fix. This reintroduces it as a standalone "Show values as" control rather than resurrecting the old per-metric aggregator-override mechanism. Now that every cell already holds a DB-correct rollup value at every scope (row total, column total, grand total), showing a cell as a fraction of one of those scopes is a pure presentational transform at render time: reuse react-pivottable's existing fractionOf template, wrapping the passthrough `cellValue` aggregator instead of `sum()`/ `count()`, and reuse the same row/column/grand-total lookup (`getAggregator`) the removed feature relied on. No new query, no touching grouping_sets, no per-metric control. Percentage display always uses a fixed percent format; per-metric custom formatters (currency, decimals, etc.) don't apply to a ratio, so formattedAggregators is skipped while a fraction mode is active.
Code Review Agent Run #f318abActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
Bot
added
change:frontend
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
|
The flagged issue is correct. The To resolve this, you can update the Example implementation for // Inside controlPanel.tsx, update the showValuesAs control configuration:
visibility: ({ controls }) => {
const rowTotals = controls?.rowTotals?.value;
const colTotals = controls?.colTotals?.value;
// Logic to filter choices based on rowTotals/colTotals
return { ... };
},I have checked the available comments on this PR, and there are no other review comments to address. Would you like me to proceed with implementing this fix in the superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #42761 +/- ##
==========================================
+ Coverage 65.77% 65.78% +0.01%
==========================================
Files 2840 2840
Lines 161983 162018 +35
Branches 37124 37148 +24
==========================================
+ Hits 106545 106585 +40
+ Misses 53373 53368 -5
Partials 2065 2065
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… totals toggle `showValuesAs` percent modes (% of row/column/grand total) divide by a rollup level that `buildGroupbyCombinations` only queried when the matching Total/subtotal display toggle was on, so picking a percent mode without enabling that toggle could crash on a missing denominator aggregator. Force the required level(s) in and fall back to a blank cell instead of throwing if one is ever still missing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rusackas
added
the
🎪 ⚡ showtime-trigger-start
github-actions
Bot
added
🎪 490356d 🚦 building
github-actions
Bot
added
🎪 490356d 🚦 deploying
… pruning test buildGroupbyCombinations iterates rows as the outer loop and columns as the inner loop, so when both the rows-collapsed and columns-collapsed levels are pruned in (the percent_total case), the rows-collapsed level pairs with every column prefix before the leaf row prefix does -- matching the row/col-only pruning tests directly above it. The new percent_total test had those two entries swapped; order carries no functional meaning downstream (transformProps/PivotData key off each row's own tags, not array position), so this fixes the expectation rather than the (correct) production code.
github-actions
Bot
added
🎪 f4a5010 🚦 building
|
🎪 Showtime deployed environment on GHA for 490356d • Environment: http://35.87.13.172:8080 (admin/admin) |
github-actions
Bot
added
🎪 f4a5010 📅 2026-08-04T23-13
Code Review Agent Run #3ef62bActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…ric fraction cross-talk Three follow-ups from review on the show-values-as-fraction feature: - `showValuesAs` was `renderTrigger: true`, but it changes which rollup levels `buildGroupbyCombinations` requests for non-additive metrics, so switching it needs a real requery, not a client-side-only re-render. - The `combineMetric` filter in `buildGroupbyCombinations` stripped the percent-mode denominator level back out whenever it landed on the opposite axis from the kept metrics-layout level, blanking percentages with Combine metrics enabled. - With multiple metrics sharing an axis, `fractionOf`'s grand/row/column total lookup hit the same metric-blind "Metric-collapse totals" slot for every metric, so all but the last metric divided by the wrong total. `fractionOf` now keeps the pushing record's own metric key in the selector so the lookup resolves to that metric's own total. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
🎪 a2f6a55 🚦 building
github-actions
Bot
added
🎪 a2f6a55 🚦 deploying
|
🎪 Showtime deployed environment on GHA for a2f6a55 • Environment: http://54.191.252.119:8080 (admin/admin) |
Code Review Agent Run #4f2494Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
2 participants
SUMMARY
#41463 (SIP-216, merged via #41184) correctly moved Pivot Table totals/subtotals to DB-computed rollup queries, fixing non-additive metrics (ratios,
COUNT_DISTINCT,AVG, etc.) that were previously wrong when re-aggregated client-side. As part of that change, the Pivot Table's old "Aggregation function" control was removed entirely, and its "Sum as Fraction of Total/Rows/Columns" options went with it.That was collateral damage, not something the SIP identified as broken: those fraction variants only ever wrapped
Sum/Count(both additive), so they were mathematically correct even before #41184, and users are now asking for the display mode back (originally raised against #41463).This reintroduces it as a standalone "Show values as" control, rather than resurrecting the removed per-metric "Aggregation function" selector. The new architecture actually makes this cleaner than before: every cell already holds a DB-correct rollup value at every scope (its own cell, row total, column total, grand total), so showing a value as a fraction of one of those scopes is now a pure presentational transform at render time. It reuses
react-pivottable's existingfractionOftemplate — the same one the old "Sum as Fraction of ..." options used — wrapping the current passthroughcellValueaggregator instead ofsum()/count(), and reusing the same row/column/grand-total lookup (getAggregator) the removed feature relied on. No new query, no touchinggrouping_sets, no per-metric control to reintroduce.Percentage display always uses a fixed percent format; a per-metric custom formatter (currency, decimals, etc.) doesn't apply to a ratio, so custom column formatters are skipped while a fraction mode is active (matching the old behavior, which also always used a fixed percent formatter regardless of the metric's own format).
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Pivot table of
birth_names(rows =state, columns =gender, metric =sum__num, row/column totals on), cycling through the new control:Actual values (default, unchanged behavior):
% of row total — every row (and the row-total column) sums to 100.0%:
% of column total — every column (and the column-total row) sums to 100.0%:
% of grand total — the grand-total corner reads 100.0%, every other cell is its share of it:
The control itself, alongside the rest of the Options panel:
TESTING INSTRUCTIONS
renderTriggercontrol).npx jest --runInBand plugins/plugin-chart-pivot-table— all 96 tests pass, including 5 new tests covering all three fraction modes plus the default/unset case.npx tsc --noEmit -p plugins/plugin-chart-pivot-table/tsconfig.jsonandpre-commit runon the changed files both pass clean.ADDITIONAL INFORMATION