You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/08/05 21:17:56 UTC
(superset) 01/02: feat(pivot-table): reintroduce show-values-as-fraction display option (#42761)
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 633f393880884e792d14fd9378d9c97d1417134e
Author: Evan Rusackas <jo...@preset.io>
AuthorDate: Wed Aug 5 14:17:30 2026 -0700
feat(pivot-table): reintroduce show-values-as-fraction display option (#42761)
Co-authored-by: Claude Opus 4.8 <no...@anthropic.com>
---
UPDATING.md | 20 +-
.../src/PivotTableChart.tsx | 2 +
.../src/plugin/controlPanel.tsx | 28 +-
.../src/plugin/transformProps.ts | 2 +
.../src/plugin/utilities.ts | 45 +++-
.../src/react-pivottable/utilities.ts | 147 +++++++++--
.../plugins/plugin-chart-pivot-table/src/types.ts | 17 ++
.../test/plugin/transformProps.test.ts | 22 ++
.../test/plugin/utilities.test.ts | 70 ++++-
.../test/react-pivottable/tableRenders.test.tsx | 240 +++++++++++++++++
...941d5352_restore_pivot_table_percent_display.py | 181 +++++++++++++
.../test_restore_pivot_table_percent_display.py | 286 +++++++++++++++++++++
12 files changed, 1024 insertions(+), 36 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 1e1d3a09126..7c46ccecbd7 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -340,9 +340,23 @@ As a result the per-table **"Aggregation function"** control (which let you pick
how totals were aggregated client-side, e.g. Sum/Average/Count) has been
removed: totals now always reflect the metric's own definition evaluated at the
total's granularity. For additive metrics (`SUM`/`COUNT`/`MIN`/`MAX`) the result
-is unchanged. Saved charts that set `aggregateFunction` will ignore it; no
-migration is required. If you previously relied on a plain sum-of-cells total
-for a non-additive metric, that specific behavior is no longer available.
+is unchanged. If you previously relied on a plain sum-of-cells total for a
+non-additive metric, that specific behavior is no longer available.
+
+The "Sum as Fraction of Total/Rows/Columns" display options are back as a
+new, standalone **"Show values as"** control (below "Combine metrics" in the
+Options panel), since those were mathematically correct even before this
+change and are unrelated to the totals-correctness fix. A DB migration
+derives the new field from any still-present `aggregateFunction` Sum-fraction
+value, so a chart that had one of those options configured picks the
+equivalent "Show values as" setting back up automatically. The "Count as
+Fraction of ..." variants are **not** migrated: they divided a record count,
+while the new control divides the metric's own value, so translating them
+automatically would silently change what the chart displays rather than
+restore it; those charts need to be manually reconfigured if the value-based
+percentage is what's wanted. Charts that used any other non-fraction
+`aggregateFunction` value (Sum, Average, Count, ...) are unaffected, since
+that specific behavior remains unavailable per the above.
### `thumbnail_url` removed from dashboard list API response
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx
index 4f3b5255130..f4b91259a42 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx
@@ -249,6 +249,7 @@ export default function PivotTableChart(props: PivotTableProps) {
columnFormats,
currencyFormats,
metricsLayout,
+ showValuesAs,
metricColorFormatters,
dateFormatters,
onContextMenu,
@@ -754,6 +755,7 @@ export default function PivotTableChart(props: PivotTableProps) {
namesMapping={verboseMap}
onContextMenu={handleContextMenu}
allowRenderHtml={allowRenderHtml}
+ showValuesAs={showValuesAs}
/>
</PivotTableWrapper>
</Styles>
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
index 6cf9b17dd8c..4107466f80d 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
@@ -33,7 +33,7 @@ import {
validateNonEmpty,
QueryFormColumn,
} from '@superset-ui/core';
-import { MetricsLayoutEnum } from '../types';
+import { MetricsLayoutEnum, ShowValuesAsEnum } from '../types';
const config: ControlPanelConfig = {
controlPanelSections: [
@@ -243,6 +243,32 @@ const config: ControlPanelConfig = {
},
},
],
+ [
+ {
+ name: 'showValuesAs',
+ config: {
+ type: 'SelectControl',
+ label: t('Show values as'),
+ default: ShowValuesAsEnum.ACTUAL,
+ // Not a renderTrigger: for non-additive metrics, a percent choice
+ // here can require a rollup level (see `buildGroupbyCombinations`)
+ // that a prior query never fetched, so switching it needs a real
+ // requery rather than a client-side-only re-render.
+ choices: [
+ [ShowValuesAsEnum.ACTUAL, t('Actual values')],
+ [ShowValuesAsEnum.PERCENT_OF_ROW, t('% of row total')],
+ [ShowValuesAsEnum.PERCENT_OF_COLUMN, t('% of column total')],
+ [ShowValuesAsEnum.PERCENT_OF_TOTAL, t('% of grand total')],
+ ],
+ description: t(
+ 'Display each cell as a percentage of its row, column, or ' +
+ 'grand total, instead of its actual value. Totals and ' +
+ 'subtotals are still computed correctly first; this only ' +
+ 'changes how they are displayed.',
+ ),
+ },
+ },
+ ],
],
},
{
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts
index b354396f2b2..ab94f125e89 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts
@@ -166,6 +166,7 @@ export default function transformProps(chartProps: ChartProps<QueryFormData>) {
valueFormat,
dateFormat,
metricsLayout,
+ showValuesAs,
conditionalFormatting,
timeGrainSqla,
currencyFormat,
@@ -242,6 +243,7 @@ export default function transformProps(chartProps: ChartProps<QueryFormData>) {
columnFormats,
currencyFormats,
metricsLayout,
+ showValuesAs,
metricColorFormatters,
dateFormatters,
onContextMenu,
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/utilities.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/utilities.ts
index 77dbe1b6887..0578818011a 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/utilities.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/utilities.ts
@@ -18,7 +18,12 @@
*/
import { QueryFormColumn, QueryFormMetric } from '@superset-ui/core';
-import { Groupby, MetricsLayoutEnum, PivotTableQueryFormData } from '../types';
+import {
+ Groupby,
+ MetricsLayoutEnum,
+ PivotTableQueryFormData,
+ ShowValuesAsEnum,
+} from '../types';
// Aggregates whose group total can be derived from per-group results
// (decomposable): summing sub-sums, counting sub-counts, min-of-mins,
@@ -189,6 +194,12 @@ export function splitGroupingSetsResult(
* - intermediate column prefix -> column subtotal -> colSubTotals
* A full-length prefix (the leaf level) is always emitted; when a dimension
* list is empty, `[]` *is* the full level and is therefore always kept.
+ *
+ * A "% of row/column/grand total" `showValuesAs` selection needs its
+ * denominator level even when the corresponding Total/subtotal display
+ * toggle is off (that toggle only controls whether the totals row/column is
+ * *rendered*; see `PivotData` in `react-pivottable/utilities.ts`), so the
+ * required collapsed level(s) are forced in regardless of the toggle.
*/
export default function buildGroupbyCombinations(
formData: PivotTableQueryFormData,
@@ -207,14 +218,26 @@ export default function buildGroupbyCombinations(
...columns.map((_, i) => columns.slice(0, i + 1)),
];
+ // "% of column total" divides each cell by its column's grand total, which
+ // is computed with all rows collapsed; "% of grand total" needs the same.
+ const needsRowsCollapsed =
+ formData.showValuesAs === ShowValuesAsEnum.PERCENT_OF_COLUMN ||
+ formData.showValuesAs === ShowValuesAsEnum.PERCENT_OF_TOTAL;
+ // "% of row total" divides each cell by its row's grand total, which is
+ // computed with all columns collapsed; "% of grand total" needs the same.
+ const needsColumnsCollapsed =
+ formData.showValuesAs === ShowValuesAsEnum.PERCENT_OF_ROW ||
+ formData.showValuesAs === ShowValuesAsEnum.PERCENT_OF_TOTAL;
+
const rowPrefixNeeded = (prefix: QueryFormColumn[]): boolean => {
if (prefix.length === rows.length) return true; // leaf / full level
- if (prefix.length === 0) return !!formData.colTotals; // bottom Total row
+ if (prefix.length === 0) return !!formData.colTotals || needsRowsCollapsed; // bottom Total row
return !!formData.rowSubTotals; // row subtotal
};
const colPrefixNeeded = (prefix: QueryFormColumn[]): boolean => {
if (prefix.length === columns.length) return true; // leaf / full level
- if (prefix.length === 0) return !!formData.rowTotals; // right Total column
+ if (prefix.length === 0)
+ return !!formData.rowTotals || needsColumnsCollapsed; // right Total column
return !!formData.colSubTotals; // column subtotal
};
@@ -227,13 +250,25 @@ export default function buildGroupbyCombinations(
);
if (formData.combineMetric) {
+ // A forced-in percent-mode denominator level (above) is collapsed on the
+ // opposite axis from a "normal" subtotal, so it can be mistaken for one
+ // and stripped back out here. Exempt it explicitly so combining metrics
+ // doesn't blank out the percent denominator.
+ const isForcedDenominatorLevel = (combination: Groupby): boolean =>
+ (needsRowsCollapsed && combination.rows.length === 0) ||
+ (needsColumnsCollapsed && combination.columns.length === 0);
+
if (formData.metricsLayout === MetricsLayoutEnum.ROWS) {
groupbyCombinations = groupbyCombinations.filter(
- combination => combination.rows.length === rows.length,
+ combination =>
+ combination.rows.length === rows.length ||
+ isForcedDenominatorLevel(combination),
);
} else {
groupbyCombinations = groupbyCombinations.filter(
- combination => combination.columns.length === columns.length,
+ combination =>
+ combination.columns.length === columns.length ||
+ isForcedDenominatorLevel(combination),
);
}
}
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts
index 8db8940e9fa..6e3ab2fa8f1 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts
@@ -748,20 +748,84 @@ const baseAggregatorTemplates = {
type
],
inner: wrapped(...Array.from(x || []))(data, rowKey, colKey),
+ // The metric this cell belongs to, and which axis carries it (see the
+ // "Metric" pseudo-dimension in PivotTableChart). With multiple
+ // metrics, the axis holding the metric is never actually empty, so
+ // collapsing it to `[]` (as the `selector` above does) would route
+ // every metric's lookup to the same shared total slot -- see
+ // `processRecord`'s "Metric-collapse totals". Keeping the metric's
+ // own key segment instead routes the lookup to the per-metric total
+ // that's already correctly split out. Updated on every push (not
+ // just the first) to stay in sync with `inner`, which likewise
+ // reflects the last-pushed record for a shared Total/corner slot --
+ // otherwise the numerator (last metric) and the denominator lookup
+ // (first metric's axis) could point at two different metrics.
+ metricAxis: undefined as
+ | { axis: 'row' | 'col'; value: string }
+ | null
+ | undefined,
push(record: PivotRecord) {
+ const metricDim = record.__metricKey as unknown as
+ | string
+ | undefined;
+ const cols = data.props.cols as string[] | undefined;
+ const rows = data.props.rows as string[] | undefined;
+ if (metricDim && cols?.includes(metricDim)) {
+ this.metricAxis = {
+ axis: 'col',
+ value: String(record[metricDim]),
+ };
+ } else if (metricDim && rows?.includes(metricDim)) {
+ this.metricAxis = {
+ axis: 'row',
+ value: String(record[metricDim]),
+ };
+ } else {
+ this.metricAxis = null;
+ }
this.inner.push(record);
},
format: fmtNonString(formatter),
value() {
- const acc = data
- .getAggregator(...Array.from(this.selector || []))
- .inner.value();
+ // `buildGroupbyCombinations` requests the denominator's rollup
+ // level whenever a percent `showValuesAs` is selected, but fall
+ // back to `null` (rendered blank) instead of throwing if it is
+ // ever missing -- e.g. a denominator aggregator with no matching
+ // rows in the response.
+ let [selRow, selCol] = (this.selector || [[], []]) as [
+ string[],
+ string[],
+ ];
+ if (this.metricAxis) {
+ if (this.metricAxis.axis === 'col' && selCol.length === 0) {
+ selCol = [this.metricAxis.value];
+ } else if (
+ this.metricAxis.axis === 'row' &&
+ selRow.length === 0
+ ) {
+ selRow = [this.metricAxis.value];
+ }
+ }
+ const denominatorAggregator = data.getAggregator(selRow, selCol);
+ if (!denominatorAggregator.inner) {
+ return null;
+ }
+ const acc = denominatorAggregator.inner.value();
if (typeof acc === 'string') {
return acc;
}
- return this.inner.value() / acc;
+ const numerator = this.inner.value();
+ // A `null` numerator (e.g. a DB-computed rollup that is null, as
+ // `cellValue`'s own comment documents) is intentionally blank in
+ // actual mode. `null` coerces to `0` under `/`, which would turn
+ // that blank into a measured `0.0%` instead of staying blank.
+ if (numerator === null) {
+ return null;
+ }
+
+ return numerator / acc;
},
getCurrencies() {
return this.inner.getCurrencies ? this.inner.getCurrencies() : [];
@@ -814,6 +878,16 @@ const aggregatorTemplates = {
...extendedAggregatorTemplates,
};
+// Maps the pivot's "Show values as" control (see ../types.ts's
+// ShowValuesAsEnum) to fractionOf's `type` parameter. `undefined`/'actual'
+// means "no wrapping" and isn't listed here.
+const FRACTION_TYPE_BY_SHOW_VALUES_AS: Record<string, 'total' | 'row' | 'col'> =
+ {
+ percent_total: 'total',
+ percent_row: 'row',
+ percent_col: 'col',
+ };
+
// default aggregators & renderers use US naming and number formatting
const aggregators = (tpl => ({
Count: tpl.count(usFmtInt),
@@ -970,32 +1044,53 @@ class PivotData {
);
const vals = this.props.vals as string[];
+ const fractionType =
+ FRACTION_TYPE_BY_SHOW_VALUES_AS[this.props.showValuesAs as string];
// Values come pre-aggregated from the database (one query per rollup level),
// so the pivot stores them verbatim via `cellValue` instead of aggregating.
- this.aggregator = cellValue(this.props.defaultFormatter as Formatter)(vals);
- this.formattedAggregators = this.props.customFormatters
- ? Object.entries(
- this.props.customFormatters as Record<
- string,
- Record<string, unknown>
- >,
- ).reduce(
- (
- acc: Record<
+ // When "Show values as" a fraction is active, wrap that passthrough with
+ // the same fractionOf template the pre-SIP-216 "Sum as Fraction of ..."
+ // aggregators used: it divides a cell's own value by the value at the
+ // requested scope (row/column/grand total), each of which is itself one
+ // of these uniformly-wrapped aggregators, so `fractionOf`'s cross-lookup
+ // (`data.getAggregator(...).inner.value()`) resolves correctly no matter
+ // which scope it's called for -- including the totals dividing by
+ // themselves to read 100%. This needs no new query and no per-metric
+ // aggregator-override control (that control is gone, see SIP.md); it's a
+ // pure display transform over values that are already DB-correct.
+ this.aggregator = fractionType
+ ? aggregatorTemplates.fractionOf(
+ cellValue(),
+ fractionType,
+ usFmtPct,
+ )(vals)
+ : cellValue(this.props.defaultFormatter as Formatter)(vals);
+ // Percentage display always uses a fixed percent format -- a per-metric
+ // custom formatter (currency, decimals, etc.) doesn't apply to a ratio.
+ this.formattedAggregators =
+ !fractionType && this.props.customFormatters
+ ? Object.entries(
+ this.props.customFormatters as Record<
string,
- Record<string, (...args: unknown[]) => Aggregator>
+ Record<string, unknown>
>,
- [key, columnFormatter],
- ) => {
- acc[key] = {};
- Object.entries(columnFormatter).forEach(([column, formatter]) => {
- acc[key][column] = cellValue(formatter as Formatter)(vals);
- });
- return acc;
- },
- {},
- )
- : false;
+ ).reduce(
+ (
+ acc: Record<
+ string,
+ Record<string, (...args: unknown[]) => Aggregator>
+ >,
+ [key, columnFormatter],
+ ) => {
+ acc[key] = {};
+ Object.entries(columnFormatter).forEach(([column, formatter]) => {
+ acc[key][column] = cellValue(formatter as Formatter)(vals);
+ });
+ return acc;
+ },
+ {},
+ )
+ : false;
this.tree = {};
this.rowKeys = [];
this.colKeys = [];
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts
index 3e3d4c145eb..87cc8f95cba 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts
@@ -50,6 +50,22 @@ export enum MetricsLayoutEnum {
COLUMNS = 'COLUMNS',
}
+/**
+ * Display mode for pivot cell values: raw values, or each value expressed as
+ * a fraction of its row/column/grand total. The division itself is a
+ * presentational transform applied at render time against the already
+ * DB-correct rollup totals (see `PivotData` in
+ * `react-pivottable/utilities.ts`), but a percent mode does need its
+ * denominator level queried even if the corresponding Total/subtotal display
+ * toggle is off; see `buildGroupbyCombinations` in `plugin/utilities.ts`.
+ */
+export enum ShowValuesAsEnum {
+ ACTUAL = 'actual',
+ PERCENT_OF_ROW = 'percent_row',
+ PERCENT_OF_COLUMN = 'percent_col',
+ PERCENT_OF_TOTAL = 'percent_total',
+}
+
/**
* One rollup level for non-additive totals: a prefix of the row dimensions and
* a prefix of the column dimensions. See `plugin/utilities.ts`.
@@ -95,6 +111,7 @@ interface PivotTableCustomizeProps {
columnFormats: JsonObject;
currencyFormats: Record<string, Currency>;
metricsLayout?: MetricsLayoutEnum;
+ showValuesAs?: ShowValuesAsEnum;
metricColorFormatters: ColorFormatters;
dateFormatters: Record<string, DateFormatter | undefined>;
legacy_order_by: QueryFormMetric[] | QueryFormMetric | null;
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/transformProps.test.ts
index a3c09d7bfc3..cfcbde32c2e 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/transformProps.test.ts
@@ -85,6 +85,28 @@ test('should pass through formData props for viz', () => {
});
});
+test('should pass through showValuesAs', () => {
+ const cp = new ChartProps<QueryFormData>({
+ formData: { ...formData, showValuesAs: 'percent_total' },
+ width: 800,
+ height: 600,
+ queriesData: [
+ {
+ data: [{ name: 'Hulk', sum__num: 1, __timestamp: 599616000000 }],
+ colnames: ['name', 'sum__num', '__timestamp'],
+ coltypes: [1, 0, 2],
+ },
+ ],
+ hooks: { setDataMask },
+ filterState: { selectedFilters: {} },
+ datasource: { verboseMap: {}, columnFormats: {} },
+ theme: supersetTheme,
+ });
+
+ const result = transformProps(cp) as ReturnType<typeof transformProps>;
+ expect(result.showValuesAs).toBe('percent_total');
+});
+
test('non-additive: transformProps splits the GROUPING SETS result by level', () => {
const gm = (col: string) => `${col}__superset_grouping`;
const localFormData = {
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/utilities.test.ts b/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/utilities.test.ts
index 5e43ea4d349..bdd3d2ea697 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/utilities.test.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/utilities.test.ts
@@ -26,7 +26,11 @@ import buildGroupbyCombinations, {
splitGroupingSetsResult,
groupingMarkerLabel,
} from '../../src/plugin/utilities';
-import { PivotTableQueryFormData, MetricsLayoutEnum } from '../../src/types';
+import {
+ PivotTableQueryFormData,
+ MetricsLayoutEnum,
+ ShowValuesAsEnum,
+} from '../../src/types';
const baseFormData = {
groupbyRows: ['row1', 'row2'],
@@ -364,6 +368,70 @@ test('pruning: empty column dims keep the [] (leaf) level regardless of rowTotal
]);
});
+test('pruning: percent_row forces the row-total (columns collapsed) level even with rowTotals off', () => {
+ const combinations = buildGroupbyCombinations({
+ ...baseFormData,
+ colTotals: false,
+ rowTotals: false,
+ colSubTotals: false,
+ rowSubTotals: false,
+ showValuesAs: ShowValuesAsEnum.PERCENT_OF_ROW,
+ });
+ expect(combinations).toEqual([
+ { rows: ['row1', 'row2'], columns: [] },
+ { rows: ['row1', 'row2'], columns: ['col1', 'col2'] },
+ ]);
+});
+
+test('pruning: percent_col forces the column-total (rows collapsed) level even with colTotals off', () => {
+ const combinations = buildGroupbyCombinations({
+ ...baseFormData,
+ colTotals: false,
+ rowTotals: false,
+ colSubTotals: false,
+ rowSubTotals: false,
+ showValuesAs: ShowValuesAsEnum.PERCENT_OF_COLUMN,
+ });
+ expect(combinations).toEqual([
+ { rows: [], columns: ['col1', 'col2'] },
+ { rows: ['row1', 'row2'], columns: ['col1', 'col2'] },
+ ]);
+});
+
+test('pruning: percent_total forces the grand-total level even with both totals off', () => {
+ const combinations = buildGroupbyCombinations({
+ ...baseFormData,
+ colTotals: false,
+ rowTotals: false,
+ colSubTotals: false,
+ rowSubTotals: false,
+ showValuesAs: ShowValuesAsEnum.PERCENT_OF_TOTAL,
+ });
+ // Rows is the outer loop and columns the inner loop (see
+ // buildGroupbyCombinations's flatMap), so the rows-collapsed level pairs
+ // with every column prefix before the leaf row prefix does.
+ expect(combinations).toEqual([
+ { rows: [], columns: [] },
+ { rows: [], columns: ['col1', 'col2'] },
+ { rows: ['row1', 'row2'], columns: [] },
+ { rows: ['row1', 'row2'], columns: ['col1', 'col2'] },
+ ]);
+});
+
+test('pruning: actual (no percent mode) does not force any extra level', () => {
+ const combinations = buildGroupbyCombinations({
+ ...baseFormData,
+ colTotals: false,
+ rowTotals: false,
+ colSubTotals: false,
+ rowSubTotals: false,
+ showValuesAs: ShowValuesAsEnum.ACTUAL,
+ });
+ expect(combinations).toEqual([
+ { rows: ['row1', 'row2'], columns: ['col1', 'col2'] },
+ ]);
+});
+
test('additiveReducerFor maps aggregates to reducers', () => {
const mk = (aggregate: string) =>
({
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx
index 971753f751f..d8e9cdc19f1 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx
@@ -760,3 +760,243 @@ test('TableRenderer renders correct number of thead and tbody sections', () => {
expect(theadEl).toBeInTheDocument();
expect(tbodyEl).toBeInTheDocument();
});
+
+/**
+ * "Show values as" a fraction (percent_row/percent_col/percent_total): a pure
+ * display transform over the already DB-correct rollup values in
+ * TAGGED_COUNT_DATA (leaf cells = 1, row/col totals = 2, grand total = 4).
+ * Reintroduces the pre-SIP-216 "Sum as Fraction of ..." display, but as a
+ * standalone control rather than resurrecting the removed per-metric
+ * "Aggregation function" selector -- see PivotData's constructor in
+ * ../../src/react-pivottable/utilities.ts.
+ */
+function getCellTexts(className: string) {
+ return screen
+ .getAllByRole('gridcell')
+ .filter(cell => cell.classList.contains(className))
+ .map(cell => cell.textContent);
+}
+
+test('TableRenderer shows values as a percentage of the grand total', () => {
+ const props = buildDefaultProps({
+ data: TAGGED_COUNT_DATA,
+ vals: ['value'],
+ tableOptions: { rowTotals: true, colTotals: true },
+ showValuesAs: 'percent_total',
+ });
+ renderWithTheme(<TableRenderer {...props} />);
+
+ // Each leaf cell is 1 out of a grand total of 4.
+ expect(getCellTexts('pvtVal')).toEqual(
+ expect.arrayContaining(['25.0%', '25.0%', '25.0%', '25.0%']),
+ );
+ // Row and column totals are 2 out of 4.
+ const rowTotalCells = getCellTexts('pvtTotal').filter(
+ text => text === '50.0%',
+ );
+ expect(rowTotalCells.length).toBeGreaterThan(0);
+ // The grand total is always 100% of itself.
+ const grandTotalCells = screen
+ .getAllByRole('gridcell')
+ .filter(cell => cell.classList.contains('pvtGrandTotal'));
+ expect(grandTotalCells).toHaveLength(1);
+ expect(grandTotalCells[0]).toHaveTextContent('100.0%');
+});
+
+test('TableRenderer shows values as a percentage of the row total', () => {
+ const props = buildDefaultProps({
+ data: TAGGED_COUNT_DATA,
+ vals: ['value'],
+ tableOptions: { rowTotals: true, colTotals: true },
+ showValuesAs: 'percent_row',
+ });
+ renderWithTheme(<TableRenderer {...props} />);
+
+ // Each leaf cell (1) is half of its row's total (2).
+ expect(getCellTexts('pvtVal')).toEqual(
+ expect.arrayContaining(['50.0%', '50.0%', '50.0%', '50.0%']),
+ );
+ // A row total is 100% of itself.
+ expect(getCellTexts('pvtTotal')).toEqual(expect.arrayContaining(['100.0%']));
+});
+
+test('TableRenderer shows values as a percentage of the column total', () => {
+ const props = buildDefaultProps({
+ data: TAGGED_COUNT_DATA,
+ vals: ['value'],
+ tableOptions: { rowTotals: true, colTotals: true },
+ showValuesAs: 'percent_col',
+ });
+ renderWithTheme(<TableRenderer {...props} />);
+
+ // Each leaf cell (1) is half of its column's total (2).
+ expect(getCellTexts('pvtVal')).toEqual(
+ expect.arrayContaining(['50.0%', '50.0%', '50.0%', '50.0%']),
+ );
+ // A column total is 100% of itself.
+ expect(getCellTexts('pvtTotal')).toEqual(expect.arrayContaining(['100.0%']));
+});
+
+/**
+ * Regression guard: when the metric pseudo-dimension collapses to the only
+ * thing on an axis (the grand-total level), each metric's own grand total
+ * must be used as the `showValuesAs` denominator -- not whichever metric's
+ * record was pushed last into the shared "Metric-collapse totals" slot (see
+ * `processRecord` in ../../src/react-pivottable/utilities.ts).
+ */
+const TAGGED_MULTI_METRIC_ON_COLUMNS = [
+ // leaf cells for metric m1 (grand total 30)
+ {
+ color: 'blue',
+ Metric: 'm1',
+ value: 10,
+ __rows: ['color'],
+ __columns: ['Metric'],
+ __metricKey: 'Metric',
+ },
+ {
+ color: 'red',
+ Metric: 'm1',
+ value: 20,
+ __rows: ['color'],
+ __columns: ['Metric'],
+ __metricKey: 'Metric',
+ },
+ // leaf cells for metric m2 (grand total 300) -- a different ratio so a
+ // cross-metric mixup produces a distinctly wrong percentage.
+ {
+ color: 'blue',
+ Metric: 'm2',
+ value: 250,
+ __rows: ['color'],
+ __columns: ['Metric'],
+ __metricKey: 'Metric',
+ },
+ {
+ color: 'red',
+ Metric: 'm2',
+ value: 50,
+ __rows: ['color'],
+ __columns: ['Metric'],
+ __metricKey: 'Metric',
+ },
+ // grand total level: rows = [], columns = [Metric]. m2 is pushed last.
+ {
+ Metric: 'm1',
+ value: 30,
+ __rows: [],
+ __columns: ['Metric'],
+ __metricKey: 'Metric',
+ },
+ {
+ Metric: 'm2',
+ value: 300,
+ __rows: [],
+ __columns: ['Metric'],
+ __metricKey: 'Metric',
+ },
+];
+
+test("TableRenderer divides percent_total by each metric's own grand total", () => {
+ const props = buildDefaultProps({
+ data: TAGGED_MULTI_METRIC_ON_COLUMNS,
+ rows: ['color'],
+ cols: ['Metric'],
+ vals: ['value'],
+ tableOptions: { rowTotals: true, colTotals: true },
+ showValuesAs: 'percent_total',
+ });
+ renderWithTheme(<TableRenderer {...props} />);
+
+ const cellTexts = getCellTexts('pvtVal');
+ // m1: 10/30 and 20/30 -- correct only if m1's own grand total (30) is used.
+ expect(cellTexts).toEqual(expect.arrayContaining(['33.3%', '66.7%']));
+ // m2: 250/300 and 50/300.
+ expect(cellTexts).toEqual(expect.arrayContaining(['83.3%', '16.7%']));
+ // A "last metric wins" bug would divide m1's cells by m2's grand total
+ // (300) instead, producing 3.3%/6.7%.
+ expect(cellTexts).not.toEqual(expect.arrayContaining(['3.3%']));
+ expect(cellTexts).not.toEqual(expect.arrayContaining(['6.7%']));
+});
+
+test('TableRenderer shows actual values when showValuesAs is unset (default)', () => {
+ const props = buildDefaultProps({
+ data: TAGGED_COUNT_DATA,
+ vals: ['value'],
+ tableOptions: { rowTotals: true, colTotals: true },
+ });
+ renderWithTheme(<TableRenderer {...props} />);
+
+ // No percent signs anywhere -- the DB-computed values render as-is.
+ const allCellTexts = screen
+ .getAllByRole('gridcell')
+ .map(cell => cell.textContent);
+ expect(allCellTexts.some(text => text?.includes('%'))).toBe(false);
+ expect(getCellTexts('pvtVal')).toEqual(
+ expect.arrayContaining(['1.00', '1.00', '1.00', '1.00']),
+ );
+});
+
+/**
+ * Regression guard: `buildGroupbyCombinations` requests the denominator's
+ * rollup level whenever a percent `showValuesAs` is active, but if a cached
+ * response predates that (e.g. a stale query result), the level a percent
+ * mode needs can be absent from the data entirely. The denominator aggregator
+ * then never receives a push and its underlying value stays `null`, which
+ * would produce `Infinity` (JS coerces `null` to `0` under `/`) rather than
+ * throwing -- the shared number formatter must render that as blank instead
+ * of leaking `Infinity%`/`NaN%` into the cell.
+ */
+test('TableRenderer renders blank instead of NaN%/Infinity% when the denominator level is missing', () => {
+ // Leaf cells and the grand total are present, but the row-total level
+ // (`__rows: ['color'], __columns: []`) that `percent_row` needs is not --
+ // simulating a response fetched before the denominator level was requested.
+ const dataMissingRowTotals = TAGGED_COUNT_DATA.filter(
+ record => !(record.__rows.length === 1 && record.__columns.length === 0),
+ );
+ const props = buildDefaultProps({
+ data: dataMissingRowTotals,
+ vals: ['value'],
+ tableOptions: { rowTotals: false, colTotals: true },
+ showValuesAs: 'percent_row',
+ });
+ renderWithTheme(<TableRenderer {...props} />);
+
+ const cellTexts = getCellTexts('pvtVal');
+ expect(cellTexts.length).toBeGreaterThan(0);
+ cellTexts.forEach(text => {
+ expect(text).not.toMatch(/NaN|Infinity/);
+ });
+ expect(cellTexts).toEqual(expect.arrayContaining(['']));
+});
+
+/**
+ * Regression guard: a per-metric custom formatter (currency, decimal
+ * precision, etc.) doesn't apply to a ratio, so `PivotData` disables
+ * `formattedAggregators` entirely whenever a fraction `showValuesAs` is
+ * active (see the constructor in ../../src/react-pivottable/utilities.ts).
+ * A cell whose group would otherwise pick up a custom formatter must still
+ * render as a plain percentage.
+ */
+test('TableRenderer ignores customFormatters while showValuesAs is a percentage', () => {
+ const customFormatters = {
+ color: {
+ blue: () => 'CUSTOM',
+ },
+ };
+ const props = buildDefaultProps({
+ data: TAGGED_COUNT_DATA,
+ vals: ['value'],
+ tableOptions: { rowTotals: true, colTotals: true },
+ showValuesAs: 'percent_row',
+ customFormatters,
+ });
+ renderWithTheme(<TableRenderer {...props} />);
+
+ const cellTexts = getCellTexts('pvtVal');
+ expect(cellTexts).not.toEqual(expect.arrayContaining(['CUSTOM']));
+ // Each leaf cell (1) is still half of its row's total (2).
+ expect(cellTexts).toEqual(
+ expect.arrayContaining(['50.0%', '50.0%', '50.0%', '50.0%']),
+ );
+});
diff --git a/superset/migrations/versions/2026-08-05_00-00_1a27941d5352_restore_pivot_table_percent_display.py b/superset/migrations/versions/2026-08-05_00-00_1a27941d5352_restore_pivot_table_percent_display.py
new file mode 100644
index 00000000000..a3757e8cd38
--- /dev/null
+++ b/superset/migrations/versions/2026-08-05_00-00_1a27941d5352_restore_pivot_table_percent_display.py
@@ -0,0 +1,181 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""restore pivot table percent display from orphaned aggregateFunction
+
+PR #41184 (SIP-216) removed the Pivot Table's per-table "Aggregation
+function" control (form_data field ``aggregateFunction``), including its
+"Sum/Count as Fraction of Total/Rows/Columns" options, in favor of
+DB-computed totals. Per that PR's own UPDATING.md note, saved charts that
+had ``aggregateFunction`` set were deliberately left as-is rather than
+migrated: "Saved charts that set aggregateFunction will ignore it; no
+migration is required." The field has been fully unused dead weight in
+``params``/``query_context`` ever since.
+
+PR #42761 reintroduces the fraction-display feature as a new, standalone
+``showValuesAs`` field. This migration derives ``showValuesAs`` from any
+still-present ``aggregateFunction`` fraction value on ``pivot_table_v2``
+charts, so a chart that had this display configured before #41184 shipped
+gets it back automatically instead of requiring someone to reopen every
+affected chart and reselect it by hand. Charts whose ``aggregateFunction``
+was a non-fraction value (Sum, Average, Count, ...) are left untouched --
+those were never broken by the removal and are out of scope here.
+
+Only the three "Sum as Fraction of ..." values are migrated. The three
+"Count as Fraction of ..." values are deliberately excluded: they divided a
+*record count* by a record count, whereas the new ``showValuesAs`` modes
+divide the metric's own *value* by that value at the requested scope (see
+``cellValue``/``fractionOf`` in
+``superset-frontend/.../plugin-chart-pivot-table/src/react-pivottable/utilities.ts``).
+Mapping "Count as Fraction" onto a value-based percent would silently change
+what a saved chart displays -- e.g. a 50/50 split by record count could
+become a 10/90 split by value -- rather than restore it, so those are left
+as un-migrated dead data, same as Sum/Average/Count.
+
+Only the ``params``/``query_context`` snapshot stored on the slice is
+patched. The stored ``query_context`` is a cache mainly used for reports/
+alerts; interactive Explore/dashboard rendering always rebuilds the query
+fresh from the current form_data, so this has no effect there. A report or
+alert that renders a migrated chart before it is next opened in Explore
+will not reflect the restored percent display in its ``query_context``
+until then, but will not error -- ``showValuesAs`` is purely a display
+transform for the additive metrics used by the vast majority of pivot
+tables.
+
+Revision ID: 1a27941d5352
+Revises: 16755d4ca4ae
+Create Date: 2026-08-05 00:00:00.000000
+
+"""
+
+from alembic import op
+from sqlalchemy import Column, Integer, String, Text
+from sqlalchemy.orm import declarative_base
+
+from superset import db
+from superset.migrations.shared.utils import paginated_update
+from superset.utils import json
+
+# revision identifiers, used by Alembic.
+revision = "1a27941d5352"
+down_revision = "16755d4ca4ae"
+
+Base = declarative_base()
+
+_VIZ_TYPE = "pivot_table_v2"
+_OLD_FIELD = "aggregateFunction"
+_NEW_FIELD = "showValuesAs"
+
+# Old `aggregateFunction` fraction values -> new `showValuesAs` enum values
+# (see ShowValuesAsEnum in superset-frontend/.../plugin-chart-pivot-table/src/types.ts).
+# "Count as Fraction of ..." values are intentionally NOT mapped here -- see the
+# module docstring for why translating them would change what the chart shows
+# rather than restore it.
+_FRACTION_MAPPING = {
+ "Sum as Fraction of Total": "percent_total",
+ "Sum as Fraction of Rows": "percent_row",
+ "Sum as Fraction of Columns": "percent_col",
+}
+
+
+class Slice(Base): # type: ignore
+ __tablename__ = "slices"
+
+ id = Column(Integer, primary_key=True)
+ viz_type = Column(String(250))
+ params = Column(Text)
+ query_context = Column(Text)
+
+
+def _migrate_params(slc: Slice) -> bool:
+ """Derive showValuesAs from an orphaned fraction aggregateFunction in
+ params. Returns True if params changed."""
+ if not slc.params:
+ return False
+ try:
+ params = json.loads(slc.params)
+ except Exception:
+ return False
+ if not isinstance(params, dict):
+ # A slice's params can be malformed (e.g. `[]`/`null`) from unrelated
+ # historical bugs; skip rather than let `.get()` raise and abort the
+ # whole migration partway through `paginated_update`'s batches.
+ return False
+
+ old_value = params.get(_OLD_FIELD)
+ new_value = _FRACTION_MAPPING.get(old_value)
+ if not new_value or _NEW_FIELD in params:
+ return False
+
+ params[_NEW_FIELD] = new_value
+ del params[_OLD_FIELD]
+ slc.params = json.dumps(params)
+ return True
+
+
+def _migrate_query_context_form_data(slc: Slice) -> bool:
+ """Mirror the same derivation into query_context.form_data, best-effort,
+ so a saved query_context snapshot stays consistent with params. Returns
+ True if query_context changed."""
+ if not slc.query_context:
+ return False
+ try:
+ qc = json.loads(slc.query_context)
+ except Exception:
+ return False
+ if not isinstance(qc, dict):
+ return False
+
+ form_data = qc.get("form_data")
+ if not isinstance(form_data, dict):
+ return False
+
+ old_value = form_data.get(_OLD_FIELD)
+ new_value = _FRACTION_MAPPING.get(old_value)
+ if not new_value or _NEW_FIELD in form_data:
+ return False
+
+ form_data[_NEW_FIELD] = new_value
+ del form_data[_OLD_FIELD]
+ slc.query_context = json.dumps(qc)
+ return True
+
+
+def upgrade() -> None:
+ bind = op.get_bind()
+ session = db.Session(bind=bind, future=True)
+
+ query = session.query(Slice).filter(Slice.viz_type == _VIZ_TYPE)
+ for slc in paginated_update(
+ query,
+ lambda current, total: print(f" {current}/{total}", end="\r"),
+ ):
+ _migrate_params(slc)
+ _migrate_query_context_form_data(slc)
+
+ session.commit()
+
+
+def downgrade() -> None:
+ # `showValuesAs` is a purely additive field: older code that doesn't know
+ # about it ignores it, exactly as `aggregateFunction` itself was left as
+ # harmless orphaned data by #41184 rather than migrated away. There is
+ # nothing to restore -- the original `aggregateFunction` value is gone by
+ # design (see module docstring), and a chart may have since had
+ # `showValuesAs` set directly through the new control rather than by this
+ # migration, so guessing a reverse mapping would risk clobbering a real
+ # user choice.
+ pass
diff --git a/tests/unit_tests/migrations/test_restore_pivot_table_percent_display.py b/tests/unit_tests/migrations/test_restore_pivot_table_percent_display.py
new file mode 100644
index 00000000000..d3c33183c71
--- /dev/null
+++ b/tests/unit_tests/migrations/test_restore_pivot_table_percent_display.py
@@ -0,0 +1,286 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Tests for migration
+``1a27941d5352_restore_pivot_table_percent_display``.
+
+Covers the two helper functions (_migrate_params,
+_migrate_query_context_form_data), the full upgrade() path across multiple
+slices, and that downgrade() is a genuine no-op.
+"""
+
+from __future__ import annotations
+
+from importlib import import_module
+from unittest.mock import patch
+
+import pytest
+from sqlalchemy import create_engine
+from sqlalchemy.orm import Session
+
+from superset.utils import json
+
+migration = import_module(
+ "superset.migrations.versions."
+ "2026-08-05_00-00_1a27941d5352_restore_pivot_table_percent_display"
+)
+
+Slice = migration.Slice
+_migrate_params = migration._migrate_params
+_migrate_query_context_form_data = migration._migrate_query_context_form_data
+_VIZ_TYPE = migration._VIZ_TYPE
+_OLD_FIELD = migration._OLD_FIELD
+_NEW_FIELD = migration._NEW_FIELD
+_FRACTION_MAPPING = migration._FRACTION_MAPPING
+
+
+@pytest.fixture
+def engine():
+ engine = create_engine("sqlite:///:memory:", future=True)
+ migration.Base.metadata.create_all(engine)
+ return engine
+
+
+# ---------------------------------------------------------------------------
+# _migrate_params
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+ ("old_value", "expected_new_value"),
+ list(_FRACTION_MAPPING.items()),
+)
+def test_migrate_params_maps_every_fraction_value(old_value, expected_new_value):
+ slc = Slice(params=json.dumps({"viz_type": _VIZ_TYPE, _OLD_FIELD: old_value}))
+ changed = _migrate_params(slc)
+ assert changed
+ result = json.loads(slc.params)
+ assert result[_NEW_FIELD] == expected_new_value
+ assert _OLD_FIELD not in result
+
+
+def test_migrate_params_noop_for_non_fraction_value():
+ params = json.dumps({"viz_type": _VIZ_TYPE, _OLD_FIELD: "Sum"})
+ slc = Slice(params=params)
+ changed = _migrate_params(slc)
+ assert not changed
+ result = json.loads(slc.params)
+ # Non-fraction aggregateFunction values are out of scope and left alone.
+ assert result[_OLD_FIELD] == "Sum"
+ assert _NEW_FIELD not in result
+
+
+@pytest.mark.parametrize(
+ "old_value",
+ [
+ "Count as Fraction of Total",
+ "Count as Fraction of Rows",
+ "Count as Fraction of Columns",
+ ],
+)
+def test_migrate_params_noop_for_count_fraction_value(old_value):
+ """Count-based fractions divided record counts, not metric values -- see
+ the module docstring for why translating them to the new value-based
+ showValuesAs modes would change what the chart displays."""
+ params = json.dumps({"viz_type": _VIZ_TYPE, _OLD_FIELD: old_value})
+ slc = Slice(params=params)
+ changed = _migrate_params(slc)
+ assert not changed
+ result = json.loads(slc.params)
+ assert result[_OLD_FIELD] == old_value
+ assert _NEW_FIELD not in result
+
+
+@pytest.mark.parametrize("malformed_params", ["[]", "null", "1", '"a string"'])
+def test_migrate_params_noop_when_params_not_dict(malformed_params):
+ """A historically malformed non-dict params value must be skipped, not
+ raise, so one bad row can't abort `superset db upgrade` partway through
+ paginated_update's batches."""
+ slc = Slice(params=malformed_params)
+ changed = _migrate_params(slc)
+ assert not changed
+ assert slc.params == malformed_params
+
+
+def test_migrate_params_noop_when_field_absent():
+ params = json.dumps({"viz_type": _VIZ_TYPE, "colTotals": True})
+ slc = Slice(params=params)
+ assert not _migrate_params(slc)
+
+
+def test_migrate_params_noop_when_already_set():
+ """Never clobber a showValuesAs the user already configured through the
+ new control, even if a stale aggregateFunction is somehow also present."""
+ params = json.dumps(
+ {
+ _OLD_FIELD: "Sum as Fraction of Total",
+ _NEW_FIELD: "percent_row",
+ }
+ )
+ slc = Slice(params=params)
+ changed = _migrate_params(slc)
+ assert not changed
+ result = json.loads(slc.params)
+ assert result[_NEW_FIELD] == "percent_row"
+
+
+def test_migrate_params_noop_when_params_empty():
+ assert not _migrate_params(Slice(params=None))
+
+
+def test_migrate_params_noop_on_invalid_json():
+ slc = Slice(params="not-json")
+ changed = _migrate_params(slc)
+ assert not changed
+ assert slc.params == "not-json"
+
+
+# ---------------------------------------------------------------------------
+# _migrate_query_context_form_data
+# ---------------------------------------------------------------------------
+
+
+def test_migrate_query_context_maps_fraction_value():
+ qc = json.dumps(
+ {
+ "form_data": {_OLD_FIELD: "Sum as Fraction of Columns"},
+ "queries": [],
+ }
+ )
+ slc = Slice(query_context=qc)
+ changed = _migrate_query_context_form_data(slc)
+ assert changed
+ result = json.loads(slc.query_context)
+ assert result["form_data"][_NEW_FIELD] == "percent_col"
+ assert _OLD_FIELD not in result["form_data"]
+
+
+def test_migrate_query_context_noop_when_form_data_missing():
+ qc = json.dumps({"queries": []})
+ slc = Slice(query_context=qc)
+ assert not _migrate_query_context_form_data(slc)
+
+
+def test_migrate_query_context_noop_when_query_context_empty():
+ assert not _migrate_query_context_form_data(Slice(query_context=None))
+
+
+@pytest.mark.parametrize("malformed_qc", ["[]", "null", "1", '"a string"'])
+def test_migrate_query_context_noop_when_query_context_not_dict(malformed_qc):
+ slc = Slice(query_context=malformed_qc)
+ changed = _migrate_query_context_form_data(slc)
+ assert not changed
+ assert slc.query_context == malformed_qc
+
+
+def test_migrate_query_context_noop_on_invalid_json():
+ slc = Slice(query_context="not-json")
+ changed = _migrate_query_context_form_data(slc)
+ assert not changed
+ assert slc.query_context == "not-json"
+
+
+# ---------------------------------------------------------------------------
+# Full upgrade() integration test
+# ---------------------------------------------------------------------------
+
+
+def test_upgrade_restores_percent_display_only_for_affected_pivot_tables(
+ engine,
+) -> None:
+ with Session(engine, future=True) as seed:
+ seed.add_all(
+ [
+ # Had a fraction display configured before #41184 -- must be
+ # restored via the new field, and the old field cleaned up.
+ Slice(
+ id=1,
+ viz_type=_VIZ_TYPE,
+ params=json.dumps(
+ {
+ "viz_type": _VIZ_TYPE,
+ _OLD_FIELD: "Sum as Fraction of Total",
+ }
+ ),
+ query_context=json.dumps(
+ {
+ "form_data": {_OLD_FIELD: "Sum as Fraction of Total"},
+ "queries": [],
+ }
+ ),
+ ),
+ # Had a non-fraction aggregateFunction -- was never broken by
+ # the removal, must be left completely untouched.
+ Slice(
+ id=2,
+ viz_type=_VIZ_TYPE,
+ params=json.dumps({"viz_type": _VIZ_TYPE, _OLD_FIELD: "Average"}),
+ query_context=None,
+ ),
+ # A different viz type that happens to reuse the same field
+ # name coincidentally -- must be left untouched.
+ Slice(
+ id=3,
+ viz_type="table",
+ params=json.dumps(
+ {
+ "viz_type": "table",
+ _OLD_FIELD: "Sum as Fraction of Rows",
+ }
+ ),
+ query_context=None,
+ ),
+ ]
+ )
+ seed.commit()
+
+ # Simulate the Alembic transaction the same way the real migration runs:
+ # bind the session to an explicit connection so paginated_update's
+ # internal commits are visible once the outer transaction closes.
+ with engine.begin() as conn:
+ upgrade_session = Session(bind=conn, future=True)
+ with (
+ patch.object(migration, "op") as mock_op,
+ patch.object(migration, "db") as mock_db,
+ ):
+ mock_op.get_bind.return_value = conn
+ mock_db.Session.return_value = upgrade_session
+ migration.upgrade()
+
+ with Session(engine, future=True) as verify:
+ slc1 = verify.get(Slice, 1)
+ params1 = json.loads(slc1.params)
+ assert params1[_NEW_FIELD] == "percent_total"
+ assert _OLD_FIELD not in params1
+ qc1 = json.loads(slc1.query_context)
+ assert qc1["form_data"][_NEW_FIELD] == "percent_total"
+ assert _OLD_FIELD not in qc1["form_data"]
+
+ slc2 = verify.get(Slice, 2)
+ params2 = json.loads(slc2.params)
+ assert params2[_OLD_FIELD] == "Average"
+ assert _NEW_FIELD not in params2
+
+ slc3 = verify.get(Slice, 3)
+ params3 = json.loads(slc3.params)
+ assert params3[_OLD_FIELD] == "Sum as Fraction of Rows"
+ assert _NEW_FIELD not in params3
+
+
+def test_downgrade_is_a_noop() -> None:
+ """downgrade() must not raise and must not be a real reverse migration --
+ see the module docstring for why nothing is restored."""
+ migration.downgrade()