From e9ab69e5f5f2585d3974ac30e5ecc78285d357f8 Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Sun, 27 Sep 2026 20:16:32 +0530 Subject: [PATCH 1/7] fix(pivot-table): blank a total that collapses mixed metrics, not sum or pick one _collapsed_metric picked the last configured metric for any rollup spanning 2+ distinct metrics on the collapsed 'Total' pseudo-dimension, so the report/export path could show a specific, plausible-looking percentage for a cell that mixes e.g. MAX(sales) with MEDIAN(msrp). #44657 already changed the interactive chart to blank that cell, since combining metrics that share no unit has no single meaningful value; this brings the export/report path in line with it. - _collapsed_metric now returns None on a mixed present, instead of reversed(metrics)'s tie-break. - pivot_df's collapse() propagates that as a genuinely blank total (was: falling back to summing across every metric). - _reduce's sum uses min_count=1 so a rollup of nothing but blanks stays blank, rather than pandas defaulting an all-NaN sum to 0. Fixes #44725. --- superset/charts/client_processing.py | 69 +++++++++++-------- .../charts/test_client_processing.py | 29 +++++++- 2 files changed, 66 insertions(+), 32 deletions(-) diff --git a/superset/charts/client_processing.py b/superset/charts/client_processing.py index 153b8c3bf1d4..6ee3126591f0 100644 --- a/superset/charts/client_processing.py +++ b/superset/charts/client_processing.py @@ -157,23 +157,20 @@ def get_metric_rollup_reducers( return reducers -def _collapsed_metric(present: list[Any], metrics: list[str]) -> Any: +def _collapsed_metric(present: list[Any]) -> Any: """ - The metric a rollup spanning `present` stands for. - - A total that collapses the metric axis is undefined in the renderer, which - resolves it to the last metric pushed into the shared slot (see the - ``metricAxis`` handling in ``react-pivottable/utilities.ts``). Mirror that - by taking the last metric in the configured order, so exported percentages - match the chart rather than summing metrics that share no unit. + The metric a rollup spanning `present` stands for, or `None` when `present` + spans more than one distinct metric. + + A total that collapses the metric axis has no single meaningful value once + it mixes distinct metrics -- e.g. a "Total" cell combining `MAX(sales)` + with `MEDIAN(msrp)`. The renderer blanks that case instead of picking one + arbitrarily (see #44657 / ``metricAxis`` handling in + ``react-pivottable/utilities.ts``); mirror it here so exported/scheduled + values agree with what the chart shows. """ distinct = set(present) - if len(distinct) == 1: - return distinct.pop() - for metric in reversed(metrics): - if metric in distinct: - return metric - return None + return distinct.pop() if len(distinct) == 1 else None def _broadcast(total: pd.Series, block: pd.DataFrame, axis: int) -> pd.DataFrame: @@ -199,9 +196,16 @@ def _reduce( reducer: str, axis: Optional[int] = None, ) -> Any: - """Apply a rollup reducer (``sum``/``min``/``max``), skipping empty cells.""" + """Apply a rollup reducer (``sum``/``min``/``max``), skipping empty cells. + + A row/column reduced down to nothing but blanks (e.g. a total collapsing + a mixed-metric slot, see ``_collapsed_metric``) stays blank: ``sum`` + otherwise defaults an all-NaN input to 0, which would resurrect the value + the caller just chose not to have. + """ method = getattr(data, reducer) - return method(axis=axis) if axis is not None else method() + kwargs = {"min_count": 1} if reducer == "sum" else {} + return method(axis=axis, **kwargs) if axis is not None else method(**kwargs) def _rollup_index( @@ -273,9 +277,7 @@ def _apply_rollup_totals( # pylint: disable=too-many-arguments,too-many-locals def metric_of(column: Any) -> Any: name = _metric_of_column(column, metric_level) - return ( - name if name in metric_names else _collapsed_metric(list(metrics), metrics) - ) + return name if name in metric_names else _collapsed_metric(list(metrics)) def lookup(row: Any, column: Any) -> tuple[bool, Any]: row_depth = row_prefix_depth.get(row, len(rows)) @@ -333,9 +335,7 @@ def _rollup_denominators( # pylint: disable=too-many-arguments,too-many-locals def metric_of(column: Any) -> Any: name = _metric_of_column(column, metric_level) - return ( - name if name in metric_names else _collapsed_metric(list(metrics), metrics) - ) + return name if name in metric_names else _collapsed_metric(list(metrics)) def denominator(row: Any, column: Any) -> tuple[bool, Any]: if mode == ShowValuesAs.PERCENT_OF_TOTAL: @@ -431,8 +431,7 @@ def _apply_show_values_as( # pylint: disable=too-many-arguments metric_of_column, denominator_selection, strict=True ) if keep and column_metric is not None - ], - metrics, + ] ) reducer = reducers.get(str(denominator_metric), DEFAULT_ROLLUP_REDUCER) if denominator_metric is not None: @@ -614,16 +613,24 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s else DEFAULT_ROLLUP_REDUCER ) - def collapse(block: pd.DataFrame) -> tuple[pd.DataFrame, str]: - """Narrow a total's source columns to one metric, and pick its reducer.""" + def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: + """Narrow a total's source columns to one metric, and pick its reducer. + + Returns `(None, reducer)` when `block` spans more than one distinct + metric: such a total has no single meaningful value, so the caller + blanks it instead of summing across metrics that share no unit + (mirrors the renderer, see #44657). + """ metric_names = set(metrics) present = [ _metric_of_column(column, totals_metric_level) for column in block.columns ] known = [metric for metric in present if metric in metric_names] - metric = _collapsed_metric(known, metrics) if known else None - if metric is None: + if not known: return block, cross_metric_reducer + metric = _collapsed_metric(known) + if metric is None: + return None, DEFAULT_ROLLUP_REDUCER keep = [column == metric for column in present] return block.loc[:, keep], reducers.get(str(metric), DEFAULT_ROLLUP_REDUCER) @@ -656,7 +663,11 @@ def collapse(block: pd.DataFrame) -> tuple[pd.DataFrame, str]: block = block.apply(pd.to_numeric, errors="coerce") if percent_mode: source, reducer = collapse(block) - subtotal = _reduce(source, reducer, axis=1) + subtotal = ( + pd.Series(np.nan, index=block.index) + if source is None + else _reduce(source, reducer, axis=1) + ) else: subtotal = pivot_v2_aggfunc_map[aggfunc](block, axis=1) depth = df.columns.nlevels - len(subgroup) - 1 diff --git a/tests/unit_tests/charts/test_client_processing.py b/tests/unit_tests/charts/test_client_processing.py index 1d3bbca6f49b..9b43be0e9fe5 100644 --- a/tests/unit_tests/charts/test_client_processing.py +++ b/tests/unit_tests/charts/test_client_processing.py @@ -1906,9 +1906,32 @@ def test_pivot_df_show_values_as_keeps_metrics_separate(): assert pivoted.loc[("US",), ("SUM(num)", "boy")] == 0.25 assert pivoted.loc[("UK",), ("MAX(num)", "boy")] == 0.375 assert pivoted.loc[("UK",), ("MAX(num)", "girl")] == 0.625 - # a total collapsing the metric axis resolves to one metric, as the - # renderer does, so it still divides by itself - assert pivoted.loc[("US",), (total_label(), "")] == 1 + # a total collapsing the metric axis mixes SUM(num) and MAX(num), which + # share no unit, so the renderer blanks it (#44657) instead of picking one + # arbitrarily (#44725) -- exported values must agree with what it shows + assert pd.isna(pivoted.loc[("US",), (total_label(), "")]) + + +def test_pivot_df_show_values_as_blanks_mixed_metric_grand_total(): + """https://github.com/apache/superset/issues/44725 + + A pivot with 2+ metrics on columns and row/column totals enabled has no + single meaningful value for the cell where both the row and column totals + meet (it would mix every configured metric). Before #44657 the renderer + picked one metric there; the export/report path must match its current, + blanked behavior rather than showing a specific, plausible-looking number. + """ + df = show_values_as_df() + df["MAX(num)"] = [1, 3, 6, 10] + pivoted = pivot_df( + df, + **{**SHOW_VALUES_AS_OPTIONS, "metrics": ["SUM(num)", "MAX(num)"]}, + show_values_as="percent_total", + ) + + assert pd.isna(pivoted.loc[("UK",), (total_label(), "")]) + assert pd.isna(pivoted.loc[("US",), (total_label(), "")]) + assert pd.isna(pivoted.loc[(total_label(),), (total_label(), "")]) def test_pivot_df_show_values_as_with_combined_metrics(): From a35d1760ce8cd0011e0a28bdbae1dd74acbb4b87 Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Tue, 29 Sep 2026 10:07:18 +0530 Subject: [PATCH 2/7] refactor(pivot-table): share the rollup metric lookup _apply_rollup_totals and _rollup_denominators each defined the same metric_of closure; replace both with a module-level _rollup_metric. --- superset/charts/client_processing.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/superset/charts/client_processing.py b/superset/charts/client_processing.py index 6ee3126591f0..0d55a167dc17 100644 --- a/superset/charts/client_processing.py +++ b/superset/charts/client_processing.py @@ -191,6 +191,16 @@ def _metric_of_column(column: Any, metric_level: int) -> Any: return column[metric_level] if isinstance(column, tuple) else column +def _rollup_metric(column: Any, metric_level: int, metrics: list[str]) -> Any: + """The metric to read from a rollup record for a cell in ``column``. + + A column carrying a total label instead of a metric name collapses the metric + axis; see ``_collapsed_metric``. + """ + name = _metric_of_column(column, metric_level) + return name if name in metrics else _collapsed_metric(metrics) + + def _reduce( data: Union[pd.DataFrame, pd.Series], reducer: str, @@ -273,11 +283,6 @@ def _apply_rollup_totals( # pylint: disable=too-many-arguments,too-many-locals cell blank. """ keyed = _rollup_index(rollup_levels) - metric_names = set(metrics) - - def metric_of(column: Any) -> Any: - name = _metric_of_column(column, metric_level) - return name if name in metric_names else _collapsed_metric(list(metrics)) def lookup(row: Any, column: Any) -> tuple[bool, Any]: row_depth = row_prefix_depth.get(row, len(rows)) @@ -289,7 +294,7 @@ def lookup(row: Any, column: Any) -> tuple[bool, Any]: record = keyed(grouped).get(key) if record is None: return False, None - return True, record.get(metric_of(column)) + return True, record.get(_rollup_metric(column, metric_level, metrics)) # Index positionally: a tuple label on a MultiIndex is ambiguous to `.loc`. for column_position, column in enumerate(df.columns): @@ -331,11 +336,6 @@ def _rollup_denominators( # pylint: disable=too-many-arguments,too-many-locals the caller fall back to a leaf-derived total. """ keyed = _rollup_index(rollup_levels) - metric_names = set(metrics) - - def metric_of(column: Any) -> Any: - name = _metric_of_column(column, metric_level) - return name if name in metric_names else _collapsed_metric(list(metrics)) def denominator(row: Any, column: Any) -> tuple[bool, Any]: if mode == ShowValuesAs.PERCENT_OF_TOTAL: @@ -356,7 +356,7 @@ def denominator(row: Any, column: Any) -> tuple[bool, Any]: record = keyed(grouped).get(key) if record is None: return False, None - return True, record.get(metric_of(column)) + return True, record.get(_rollup_metric(column, metric_level, metrics)) resolved = [[denominator(row, column) for column in df.columns] for row in df.index] values = pd.DataFrame( From 486e5971b91cb1000708bbcc5b006904a95ee523 Mon Sep 17 00:00:00 2001 From: Matt Fitzgerald Date: Thu, 15 Oct 2026 11:20:27 +1100 Subject: [PATCH 3/7] fix(pivot): blank mixed-metric totals in actual value exports --- superset/charts/client_processing.py | 33 ++-- .../charts/test_client_processing.py | 160 ++++++++++++------ 2 files changed, 126 insertions(+), 67 deletions(-) diff --git a/superset/charts/client_processing.py b/superset/charts/client_processing.py index b8a19dcb8d28..8697c0527bad 100644 --- a/superset/charts/client_processing.py +++ b/superset/charts/client_processing.py @@ -157,7 +157,7 @@ def get_metric_rollup_reducers( return reducers -def _collapsed_metric(present: list[Any]) -> Any: +def _collapsed_metric(present: list[Any] | set[Any]) -> Any: """ The metric a rollup spanning `present` stands for, or `None` when `present` spans more than one distinct metric. @@ -191,7 +191,7 @@ def _metric_of_column(column: Any, metric_level: int) -> Any: return column[metric_level] if isinstance(column, tuple) else column -def _rollup_metric(column: Any, metric_level: int, metrics: list[str]) -> Any: +def _rollup_metric(column: Any, metric_level: int, metrics: set[str]) -> Any: """The metric to read from a rollup record for a cell in ``column``. A column carrying a total label instead of a metric name collapses the metric @@ -283,6 +283,7 @@ def _apply_rollup_totals( # pylint: disable=too-many-arguments,too-many-locals cell blank. """ keyed = _rollup_index(rollup_levels) + metric_names = set(metrics) def lookup(row: Any, column: Any) -> tuple[bool, Any]: row_depth = row_prefix_depth.get(row, len(rows)) @@ -294,7 +295,7 @@ def lookup(row: Any, column: Any) -> tuple[bool, Any]: record = keyed(grouped).get(key) if record is None: return False, None - return True, record.get(_rollup_metric(column, metric_level, metrics)) + return True, record.get(_rollup_metric(column, metric_level, metric_names)) # Index positionally: a tuple label on a MultiIndex is ambiguous to `.loc`. for column_position, column in enumerate(df.columns): @@ -336,6 +337,7 @@ def _rollup_denominators( # pylint: disable=too-many-arguments,too-many-locals the caller fall back to a leaf-derived total. """ keyed = _rollup_index(rollup_levels) + metric_names = set(metrics) def denominator(row: Any, column: Any) -> tuple[bool, Any]: if mode == ShowValuesAs.PERCENT_OF_TOTAL: @@ -356,7 +358,7 @@ def denominator(row: Any, column: Any) -> tuple[bool, Any]: record = keyed(grouped).get(key) if record is None: return False, None - return True, record.get(_rollup_metric(column, metric_level, metrics)) + return True, record.get(_rollup_metric(column, metric_level, metric_names)) resolved = [[denominator(row, column) for column in df.columns] for row in df.index] values = pd.DataFrame( @@ -634,6 +636,7 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: keep = [column == metric for column in present] return block.loc[:, keep], reducers.get(str(metric), DEFAULT_ROLLUP_REDUCER) + mixed_metric_totals: list[Any] = [] if show_rows_total: # add subtotal for each group and overall total; we start from the # overall group, and iterate deeper into subgroups @@ -661,13 +664,14 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: # to a number. The row totals below already coerce; do the # same here so a total means the same thing on both axes. block = block.apply(pd.to_numeric, errors="coerce") - if percent_mode: - source, reducer = collapse(block) - subtotal = ( - pd.Series(np.nan, index=block.index) - if source is None - else _reduce(source, reducer, axis=1) - ) + source, reducer = collapse(block) + mixed_metrics = ( + source is None and aggfunc != CURRENCY_CONTEXT_AGGREGATION + ) + if mixed_metrics: + subtotal = pd.Series(np.nan, index=block.index) + elif percent_mode and source is not None: + subtotal = _reduce(source, reducer, axis=1) else: subtotal = pivot_v2_aggfunc_map[aggfunc](block, axis=1) depth = df.columns.nlevels - len(subgroup) - 1 @@ -676,6 +680,8 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: # insert column after subgroup df.insert(int(slice_.stop), subtotal_name, subtotal) inserted_columns.append(subtotal_name) + if mixed_metrics: + mixed_metric_totals.append(subtotal_name) column_prefix_depth[subtotal_name] = ( level if combine_metrics else max(0, level - 1) ) @@ -718,6 +724,11 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: inserted_rows.append(subtotal.name) row_prefix_depth[subtotal.name] = level + # Reducing an all-blank mixed-metric column can produce 0 (sum/count). + # Keep these slots blank, including the grand-total corner, in every mode. + for column in mixed_metric_totals: + df[column] = np.nan + # A non-additive metric's totals come from the database rollups the chart # computed, in Actual Values mode as in the percent modes: reducing the # already-aggregated leaf cells again (mean of means, distinct-count of diff --git a/tests/unit_tests/charts/test_client_processing.py b/tests/unit_tests/charts/test_client_processing.py index a51d808c4af9..d34c9c7e9251 100644 --- a/tests/unit_tests/charts/test_client_processing.py +++ b/tests/unit_tests/charts/test_client_processing.py @@ -25,6 +25,7 @@ from sqlalchemy.orm.session import Session from superset.charts.client_processing import ( + _reduce, apply_client_processing, apply_pivot_number_formats, format_column, @@ -245,7 +246,7 @@ def test_pivot_df_no_cols_no_rows_two_metrics(): == f""" | | ('SUM(num)',) | ('MAX(num)',) | ('{_("Total")} (Sum)',) | |:-----------------|----------------:|----------------:|-------------------:| -| ('{_("Total")} (Sum)',) | 80679663 | 37296 | 80716959 | +| ('{_("Total")} (Sum)',) | 8.06797e+07 | 37296 | nan | """.strip() ) @@ -356,9 +357,9 @@ def test_pivot_df_single_row_two_metrics(): == f""" | | ('SUM(num)',) | ('MAX(num)',) | ('{_("Total")} (Sum)',) | |:-----------------|----------------:|----------------:|-------------------:| -| ('boy',) | 47123 | 1280 | 48403 | -| ('girl',) | 118065 | 2588 | 120653 | -| ('{_("Total")} (Sum)',) | 165188 | 3868 | 169056 | +| ('boy',) | 47123 | 1280 | nan | +| ('girl',) | 118065 | 2588 | nan | +| ('{_("Total")} (Sum)',) | 165188 | 3868 | nan | """.strip() ) @@ -382,11 +383,11 @@ def test_pivot_df_single_row_two_metrics(): |:-------------------------|-------------------:| | ('SUM(num)', 'boy') | 47123 | | ('SUM(num)', 'girl') | 118065 | -| ('SUM(num)', 'Subtotal') | 165188 | +| ('SUM(num)', '{_("Subtotal")}') | 165188 | | ('MAX(num)', 'boy') | 1280 | | ('MAX(num)', 'girl') | 2588 | -| ('MAX(num)', 'Subtotal') | 3868 | -| ('{_("Total")} (Sum)', '') | 169056 | +| ('MAX(num)', '{_("Subtotal")}') | 3868 | +| ('{_("Total")} (Sum)', '') | nan | """.strip() ) @@ -410,11 +411,11 @@ def test_pivot_df_single_row_two_metrics(): |:---------------------|-------------------:| | ('boy', 'SUM(num)') | 47123 | | ('boy', 'MAX(num)') | 1280 | -| ('boy', 'Subtotal') | 48403 | +| ('boy', '{_("Subtotal")}') | nan | | ('girl', 'SUM(num)') | 118065 | | ('girl', 'MAX(num)') | 2588 | -| ('girl', 'Subtotal') | 120653 | -| ('{_("Total")} (Sum)', '') | 169056 | +| ('girl', '{_("Subtotal")}') | nan | +| ('{_("Total")} (Sum)', '') | nan | """.strip() ) @@ -525,9 +526,9 @@ def test_pivot_df_single_row_null_values(): == """ | | ('SUM(num)',) | ('MAX(num)',) | ('Total (Sum)',) | |:-----------------|----------------:|----------------:|-------------------:| -| ('boy',) | nan | nan | 0 | -| ('girl',) | 118065 | 2588 | 120653 | -| ('Total (Sum)',) | 118065 | 2588 | 120653 | +| ('boy',) | nan | nan | nan | +| ('girl',) | 118065 | 2588 | nan | +| ('Total (Sum)',) | 118065 | 2588 | nan | """.strip() ) @@ -576,11 +577,11 @@ def test_pivot_df_single_row_null_values(): |:---------------------|-------------------:| | ('boy', 'SUM(num)') | nan | | ('boy', 'MAX(num)') | nan | -| ('boy', 'Subtotal') | 0 | +| ('boy', '{_("Subtotal")}') | nan | | ('girl', 'SUM(num)') | 118065 | | ('girl', 'MAX(num)') | 2588 | -| ('girl', 'Subtotal') | 120653 | -| ('{_("Total")} (Sum)', '') | 120653 | +| ('girl', '{_("Subtotal")}') | nan | +| ('{_("Total")} (Sum)', '') | nan | """.strip() ) @@ -692,9 +693,9 @@ def test_pivot_df_single_row_null_mix_values_strings(): == """ | | ('SUM(num)',) | ('MAX(num)',) | ('Total (Sum)',) | |:-----------------|:----------------|----------------:|-------------------:| -| ('boy',) | NULL | nan | 0 | -| ('girl',) | 118065 | 2588 | 120653 | -| ('Total (Sum)',) | 118065.0 | 2588 | 120653 | +| ('boy',) | NULL | nan | nan | +| ('girl',) | 118065 | 2588 | nan | +| ('Total (Sum)',) | 118065.0 | 2588 | nan | """.strip() ) @@ -718,11 +719,11 @@ def test_pivot_df_single_row_null_mix_values_strings(): |:---------------------|:-------------------| | ('boy', 'SUM(num)') | NULL | | ('boy', 'MAX(num)') | nan | -| ('boy', 'Subtotal') | 0.0 | +| ('boy', 'Subtotal') | nan | | ('girl', 'SUM(num)') | 118065 | | ('girl', 'MAX(num)') | 2588.0 | -| ('girl', 'Subtotal') | 120653.0 | -| ('Total (Sum)', '') | 120653.0 | +| ('girl', 'Subtotal') | nan | +| ('Total (Sum)', '') | nan | """.strip() ) @@ -862,7 +863,7 @@ def test_pivot_df_single_row_null_mix_values_numbers(): | ('MAX(num)', 'boy') | nan | | ('MAX(num)', 'girl') | 2588 | | ('MAX(num)', 'Subtotal') | 2588 | -| ('Total (Sum)', '') | 120674 | +| ('Total (Sum)', '') | nan | """.strip() ) @@ -886,11 +887,11 @@ def test_pivot_df_single_row_null_mix_values_numbers(): |:---------------------|-------------------:| | ('boy', 'SUM(num)') | 21 | | ('boy', 'MAX(num)') | nan | -| ('boy', 'Subtotal') | 21 | +| ('boy', '{_("Subtotal")}') | nan | | ('girl', 'SUM(num)') | 118065 | | ('girl', 'MAX(num)') | 2588 | -| ('girl', 'Subtotal') | 120653 | -| ('{_("Total")} (Sum)', '') | 120674 | +| ('girl', '{_("Subtotal")}') | nan | +| ('{_("Total")} (Sum)', '') | nan | """.strip() ) @@ -1091,15 +1092,15 @@ def test_pivot_df_complex(): == """ | | ('SUM(num)', 'CA') | ('SUM(num)', 'FL') | ('SUM(num)', 'Subtotal') | ('MAX(num)', 'CA') | ('MAX(num)', 'FL') | ('MAX(num)', 'Subtotal') | ('Total (Sum)', '') | |:---------------------|---------------------:|---------------------:|---------------------------:|---------------------:|---------------------:|---------------------------:|----------------------:| -| ('boy', 'Edward') | 31290 | 9395 | 40685 | 1280 | 389 | 1669 | 42354 | -| ('boy', 'Tony') | 3765 | 2673 | 6438 | 598 | 247 | 845 | 7283 | -| ('boy', 'Subtotal') | 35055 | 12068 | 47123 | 1878 | 636 | 2514 | 49637 | -| ('girl', 'Amy') | 45426 | 14740 | 60166 | 2227 | 854 | 3081 | 63247 | -| ('girl', 'Cindy') | 14149 | 1218 | 15367 | 842 | 217 | 1059 | 16426 | -| ('girl', 'Dawn') | 11403 | 5089 | 16492 | 1157 | 461 | 1618 | 18110 | -| ('girl', 'Sophia') | 18859 | 7181 | 26040 | 2588 | 1187 | 3775 | 29815 | -| ('girl', 'Subtotal') | 89837 | 28228 | 118065 | 6814 | 2719 | 9533 | 127598 | -| ('Total (Sum)', '') | 124892 | 40296 | 165188 | 8692 | 3355 | 12047 | 177235 | +| ('boy', 'Edward') | 31290 | 9395 | 40685 | 1280 | 389 | 1669 | nan | +| ('boy', 'Tony') | 3765 | 2673 | 6438 | 598 | 247 | 845 | nan | +| ('boy', 'Subtotal') | 35055 | 12068 | 47123 | 1878 | 636 | 2514 | nan | +| ('girl', 'Amy') | 45426 | 14740 | 60166 | 2227 | 854 | 3081 | nan | +| ('girl', 'Cindy') | 14149 | 1218 | 15367 | 842 | 217 | 1059 | nan | +| ('girl', 'Dawn') | 11403 | 5089 | 16492 | 1157 | 461 | 1618 | nan | +| ('girl', 'Sophia') | 18859 | 7181 | 26040 | 2588 | 1187 | 3775 | nan | +| ('girl', 'Subtotal') | 89837 | 28228 | 118065 | 6814 | 2719 | 9533 | nan | +| ('Total (Sum)', '') | 124892 | 40296 | 165188 | 8692 | 3355 | 12047 | nan | """.strip() # noqa: E501 ) @@ -1189,11 +1190,11 @@ def test_pivot_df_complex(): |:--------------------|--------------------:|------------------:|----------------------:|------------------:|--------------------:|-------------------:|---------------------:|-----------------------:|----------------------:| | ('CA', 'SUM(num)') | 31290 | 3765 | 35055 | 45426 | 14149 | 11403 | 18859 | 89837 | 124892 | | ('CA', 'MAX(num)') | 1280 | 598 | 1878 | 2227 | 842 | 1157 | 2588 | 6814 | 8692 | -| ('CA', 'Subtotal') | 32570 | 4363 | 36933 | 47653 | 14991 | 12560 | 21447 | 96651 | 133584 | +| ('CA', 'Subtotal') | nan | nan | nan | nan | nan | nan | nan | nan | nan | | ('FL', 'SUM(num)') | 9395 | 2673 | 12068 | 14740 | 1218 | 5089 | 7181 | 28228 | 40296 | | ('FL', 'MAX(num)') | 389 | 247 | 636 | 854 | 217 | 461 | 1187 | 2719 | 3355 | -| ('FL', 'Subtotal') | 9784 | 2920 | 12704 | 15594 | 1435 | 5550 | 8368 | 30947 | 43651 | -| ('Total (Sum)', '') | 42354 | 7283 | 49637 | 63247 | 16426 | 18110 | 29815 | 127598 | 177235 | +| ('FL', 'Subtotal') | nan | nan | nan | nan | nan | nan | nan | nan | nan | +| ('Total (Sum)', '') | nan | nan | nan | nan | nan | nan | nan | nan | nan | """.strip() # noqa: E501 ) @@ -1381,9 +1382,8 @@ def test_pivot_df_multi_column(): == """ | | ('SUM(num)', 'boy') | ('SUM(num)', 'girl') | ('SUM(num)', 'Subtotal') | ('MAX(num)', 'boy') | ('MAX(num)', 'girl') | ('MAX(num)', 'Subtotal') | ('Total (Sum)', '') | |:-----------------|----------------------:|-----------------------:|---------------------------:|----------------------:|-----------------------:|---------------------------:|----------------------:| -| ('CA',) | 35055 | 89837 | 124892 | 1878 | 6814 | 8692 | 133584 | -| ('Total (Sum)',) | 12068 | 28228 | 40296 | 636 | 2719 | 3355 | 43651 | - +| ('CA',) | 35055 | 89837 | 124892 | 1878 | 6814 | 8692 | nan | +| ('Total (Sum)',) | 12068 | 28228 | 40296 | 636 | 2719 | 3355 | nan | """.strip() # noqa: E501 ) @@ -1453,11 +1453,11 @@ def test_pivot_df_multi_column(): |:---------------------|----------:|-------------------:| | ('boy', 'SUM(num)') | 35055 | 12068 | | ('boy', 'MAX(num)') | 1878 | 636 | -| ('boy', 'Subtotal') | 36933 | 12704 | +| ('boy', 'Subtotal') | nan | nan | | ('girl', 'SUM(num)') | 89837 | 28228 | | ('girl', 'MAX(num)') | 6814 | 2719 | -| ('girl', 'Subtotal') | 96651 | 30947 | -| ('Total (Sum)', '') | 133584 | 43651 | +| ('girl', 'Subtotal') | nan | nan | +| ('Total (Sum)', '') | nan | nan | """.strip() ) @@ -1677,16 +1677,16 @@ def test_pivot_df_complex_null_values(): == """ | | ('SUM(num)', nan) | ('SUM(num)', 'Subtotal') | ('MAX(num)', nan) | ('MAX(num)', 'Subtotal') | ('Total (Sum)', '') | |:---------------------|--------------------:|---------------------------:|--------------------:|---------------------------:|----------------------:| -| ('boy', 'Edward') | 40685 | 40685 | 1669 | 1669 | 42354 | -| ('boy', 'Tony') | 6438 | 6438 | 845 | 845 | 7283 | -| ('boy', 'Subtotal') | 47123 | 47123 | 2514 | 2514 | 49637 | -| ('girl', 'Amy') | 60166 | 60166 | 3081 | 3081 | 63247 | -| ('girl', 'Cindy') | 15367 | 15367 | 1059 | 1059 | 16426 | -| ('girl', 'Dawn') | 16492 | 16492 | 1618 | 1618 | 18110 | -| ('girl', 'Sophia') | 26040 | 26040 | 3775 | 3775 | 29815 | -| ('girl', 'Subtotal') | 118065 | 118065 | 9533 | 9533 | 127598 | -| ('Total (Sum)', '') | 165188 | 165188 | 12047 | 12047 | 177235 | - """.strip() # noqa: E501 +| ('boy', 'Edward') | 40685 | 40685 | 1669 | 1669 | nan | +| ('boy', 'Tony') | 6438 | 6438 | 845 | 845 | nan | +| ('boy', 'Subtotal') | 47123 | 47123 | 2514 | 2514 | nan | +| ('girl', 'Amy') | 60166 | 60166 | 3081 | 3081 | nan | +| ('girl', 'Cindy') | 15367 | 15367 | 1059 | 1059 | nan | +| ('girl', 'Dawn') | 16492 | 16492 | 1618 | 1618 | nan | +| ('girl', 'Sophia') | 26040 | 26040 | 3775 | 3775 | nan | +| ('girl', 'Subtotal') | 118065 | 118065 | 9533 | 9533 | nan | +| ('Total (Sum)', '') | 165188 | 165188 | 12047 | 12047 | nan | + """.strip() # noqa: E501 ) # apply_metrics_on_rows @@ -1775,8 +1775,8 @@ def test_pivot_df_complex_null_values(): |:--------------------|--------------------:|------------------:|----------------------:|------------------:|--------------------:|-------------------:|---------------------:|-----------------------:|----------------------:| | (nan, 'SUM(num)') | 40685 | 6438 | 47123 | 60166 | 15367 | 16492 | 26040 | 118065 | 165188 | | (nan, 'MAX(num)') | 1669 | 845 | 2514 | 3081 | 1059 | 1618 | 3775 | 9533 | 12047 | -| (nan, 'Subtotal') | 42354 | 7283 | 49637 | 63247 | 16426 | 18110 | 29815 | 127598 | 177235 | -| ('Total (Sum)', '') | 42354 | 7283 | 49637 | 63247 | 16426 | 18110 | 29815 | 127598 | 177235 | +| (nan, 'Subtotal') | nan | nan | nan | nan | nan | nan | nan | nan | nan | +| ('Total (Sum)', '') | nan | nan | nan | nan | nan | nan | nan | nan | nan | """.strip() # noqa: E501 ) @@ -1934,6 +1934,48 @@ def test_pivot_df_show_values_as_blanks_mixed_metric_grand_total(): assert pd.isna(pivoted.loc[(total_label(),), (total_label(), "")]) +@pytest.mark.parametrize("layout", ["COLUMNS", "ROWS"]) +@pytest.mark.parametrize("with_rollups", [False, True]) +@pytest.mark.parametrize("mode", [None, "percent_row", "percent_col", "percent_total"]) +def test_pivot_table_v2_blanks_mixed_metric_totals( + layout: str, with_rollups: bool, mode: str | None +): + """Mixed totals stay blank in both layouts, with or without GROUPING SETS.""" + if with_rollups: + df = grouping_sets_df() + df["MAX(num)"] = [1, 6, 6, 1, 6, 6] + metrics = ["AVG(num)", "MAX(num)"] + else: + df = show_values_as_df() + df["MAX(num)"] = [1, 3, 6, 10] + metrics = ["SUM(num)", "MAX(num)"] + form_data = { + "groupbyRows": ["nation"], + "groupbyColumns": ["gender"], + "metrics": metrics, + "metricsLayout": layout, + "rowTotals": True, + "colTotals": True, + "showValuesAs": mode, + } + pivoted = pivot_table_v2(df, form_data, apply_number_format=False) + if layout == "ROWS": + mixed = pivoted.loc[(total_label(), ""), :] + leaf = pivoted.loc[(metrics[0], "US"), ("boy",)] + metric_total = pivoted.loc[(metrics[0], "Subtotal"), (total_label(),)] + else: + mixed = pivoted.loc[:, (total_label(), "")] + leaf = pivoted.loc[("US",), (metrics[0], "boy")] + metric_total = pivoted.loc[(total_label(),), (metrics[0], "Subtotal")] + # Includes the grand-total corner: all-blank sums must not revive it as 0. + assert mixed.isna().all() + assert pd.notna(leaf) + assert pd.notna(metric_total) + if mode is None: + assert leaf == 10 + assert metric_total == (19 if with_rollups else 80) + + def test_pivot_df_show_values_as_with_combined_metrics(): """`combineMetric` moves the metric to the lowest column level.""" df = show_values_as_df() @@ -4657,3 +4699,9 @@ def test_apply_client_processing_csv_format_pivot_table_multiple_rows(): ("London", "Consumer"), ("London", "Corporate"), } + + +@pytest.mark.parametrize("reducer", ["sum", "min", "max"]) +def test_pivot_rollup_reducer_preserves_blank_and_zero(reducer: str): + assert pd.isna(_reduce(pd.Series([float("nan"), float("nan")]), reducer)) + assert _reduce(pd.Series([0, float("nan")]), reducer) == 0 From 63f63d0cbd873fbc4c02eeba9c8d197f56b77d77 Mon Sep 17 00:00:00 2001 From: Matt Fitzgerald Date: Sat, 17 Oct 2026 11:05:18 +1100 Subject: [PATCH 4/7] fix(pivot-table): preserve integer exports with blank totals --- UPDATING.md | 13 +++ superset/charts/client_processing.py | 42 +++++----- .../charts/test_client_processing.py | 79 +++++++++++++++++-- 3 files changed, 108 insertions(+), 26 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 5eddffc2f77e..58d038a98e5e 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,19 @@ assists people when migrating to a new version. ## Next +### Pivot Table mixed-metric export totals + +CSV/Excel exports and scheduled CSV/text reports now leave Total and Subtotal +cells blank when they combine different metrics, including two SUM metrics. +Metric-specific values and totals remain available. This also applies with +Combine metrics and Transpose pivot enabled. Previously these cells could show +an arbitrary metric's value or a sum across unrelated metrics. + +If downstream processing consumes these totals, update it to handle empty +cells or define a single metric that explicitly computes the desired combined +value. This change does not restore result-aggregation parity between exports +and the browser. + - **Semantic NULL filters and provider SDK upgrade.** Semantic filters normalize UI NULL/empty-string selections. Mixed positive NULL membership uses `OrFilter`, which every semantic provider must render; previously NULL rows could be diff --git a/superset/charts/client_processing.py b/superset/charts/client_processing.py index 8697c0527bad..f5a965cb89b3 100644 --- a/superset/charts/client_processing.py +++ b/superset/charts/client_processing.py @@ -164,8 +164,8 @@ def _collapsed_metric(present: list[Any] | set[Any]) -> Any: A total that collapses the metric axis has no single meaningful value once it mixes distinct metrics -- e.g. a "Total" cell combining `MAX(sales)` - with `MEDIAN(msrp)`. The renderer blanks that case instead of picking one - arbitrarily (see #44657 / ``metricAxis`` handling in + with `MEDIAN(msrp)`. The browser change in #44657 blanks that case instead + of picking one arbitrarily (see ``metricAxis`` handling in ``react-pivottable/utilities.ts``); mirror it here so exported/scheduled values agree with what the chart shows. """ @@ -399,8 +399,8 @@ def _apply_show_values_as( # pylint: disable=too-many-arguments - A total is rolled up within a single metric, so a cell is never divided by a total that mixes in another metric, and each metric uses its own reducer -- a MIN/MAX metric divides by the row's minimum/maximum rather - than its sum. A total that collapses the metric axis resolves to a single - metric the way the renderer does; see ``_collapsed_metric``. + than its sum. A total that combines distinct metrics stays blank; see + ``_collapsed_metric``. A zero denominator yields NaN (blank) rather than infinity, matching ``pandas_postprocessing.pivot``'s ``show_values_as``. @@ -420,6 +420,8 @@ def _apply_show_values_as( # pylint: disable=too-many-arguments derived = pd.DataFrame(np.nan, index=numeric.index, columns=numeric.columns) for metric in dict.fromkeys(metric_of_column): selection = np.array([column == metric for column in metric_of_column]) + # Mixed-metric numerators are blank, so the fallback cross-metric + # denominator cannot produce a displayed value. denominator_selection = ( selection if metric is not None else np.ones(len(selection), dtype=bool) ) @@ -607,14 +609,16 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s # still divides by itself: a MAX metric's row total is the row's maximum, # and dividing that maximum by itself reads 100%. totals_metric_level = df.columns.nlevels - 1 if combine_metrics else 0 - # A column carrying a total label rather than a metric name rolls up - # everything; with no metric to resolve it to, it sums. + # Single-metric totals retain their reducer. Mixed-metric totals are + # blanked before reduction; the fallback cannot make them meaningful. cross_metric_reducer = ( reducers.get(metrics[0], DEFAULT_ROLLUP_REDUCER) if len(set(metrics)) == 1 else DEFAULT_ROLLUP_REDUCER ) + metric_names = set(metrics) + def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: """Narrow a total's source columns to one metric, and pick its reducer. @@ -623,7 +627,6 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: blanks it instead of summing across metrics that share no unit (mirrors the renderer, see #44657). """ - metric_names = set(metrics) present = [ _metric_of_column(column, totals_metric_level) for column in block.columns ] @@ -670,7 +673,7 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: ) if mixed_metrics: subtotal = pd.Series(np.nan, index=block.index) - elif percent_mode and source is not None: + elif percent_mode: subtotal = _reduce(source, reducer, axis=1) else: subtotal = pivot_v2_aggfunc_map[aggfunc](block, axis=1) @@ -703,7 +706,7 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: ) raise - subtotal_values = df.iloc[slice_, :] + subtotal_values = df.iloc[slice_, :].drop(columns=mixed_metric_totals) if aggfunc != CURRENCY_CONTEXT_AGGREGATION: subtotal_values = subtotal_values.apply( pd.to_numeric, errors="coerce" @@ -719,16 +722,15 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: subtotal.name = tuple([*subgroup, total, *([""] * depth)]) # noqa: C409 # insert row after subgroup df = pd.concat( - [df[: slice_.stop], subtotal.to_frame().T, df[slice_.stop :]] + [ + df[: slice_.stop], + subtotal.to_frame().T.reindex(columns=df.columns), + df[slice_.stop :], + ] ) inserted_rows.append(subtotal.name) row_prefix_depth[subtotal.name] = level - # Reducing an all-blank mixed-metric column can produce 0 (sum/count). - # Keep these slots blank, including the grand-total corner, in every mode. - for column in mixed_metric_totals: - df[column] = np.nan - # A non-additive metric's totals come from the database rollups the chart # computed, in Actual Values mode as in the percent modes: reducing the # already-aggregated leaf cells again (mean of means, distinct-count of @@ -774,13 +776,13 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: else None, ) - # if we want to apply the metrics on the rows we need to pivot the - # dataframe back + # Replace placeholders before transposing, while column dtypes are intact. + df.replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True) if apply_metrics_on_rows: - df = df.T + # Mixed blank columns must not coerce integer cells through float64 + # during transposition (which would round integers above 2**53). + df = df.astype(object).T if mixed_metric_totals else df.T - # replace the remaining temporary placeholder string for np.nan after pivoting - df.replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True) df.rename( index={"SUPERSET_PANDAS_NAN": np.nan}, columns={"SUPERSET_PANDAS_NAN": np.nan}, diff --git a/tests/unit_tests/charts/test_client_processing.py b/tests/unit_tests/charts/test_client_processing.py index d34c9c7e9251..a94fcfdadb54 100644 --- a/tests/unit_tests/charts/test_client_processing.py +++ b/tests/unit_tests/charts/test_client_processing.py @@ -1937,8 +1937,9 @@ def test_pivot_df_show_values_as_blanks_mixed_metric_grand_total(): @pytest.mark.parametrize("layout", ["COLUMNS", "ROWS"]) @pytest.mark.parametrize("with_rollups", [False, True]) @pytest.mark.parametrize("mode", [None, "percent_row", "percent_col", "percent_total"]) +@pytest.mark.parametrize("aggregation", ["Sum", "Average"]) def test_pivot_table_v2_blanks_mixed_metric_totals( - layout: str, with_rollups: bool, mode: str | None + layout: str, with_rollups: bool, mode: str | None, aggregation: str ): """Mixed totals stay blank in both layouts, with or without GROUPING SETS.""" if with_rollups: @@ -1957,23 +1958,27 @@ def test_pivot_table_v2_blanks_mixed_metric_totals( "rowTotals": True, "colTotals": True, "showValuesAs": mode, + "aggregateFunction": aggregation, } pivoted = pivot_table_v2(df, form_data, apply_number_format=False) + total = total_label() if mode else f"{_('Total')} ({aggregation})" if layout == "ROWS": - mixed = pivoted.loc[(total_label(), ""), :] + mixed = pivoted.loc[(total, ""), :] leaf = pivoted.loc[(metrics[0], "US"), ("boy",)] - metric_total = pivoted.loc[(metrics[0], "Subtotal"), (total_label(),)] + metric_total = pivoted.loc[(metrics[0], "Subtotal"), (total,)] else: - mixed = pivoted.loc[:, (total_label(), "")] + mixed = pivoted.loc[:, (total, "")] leaf = pivoted.loc[("US",), (metrics[0], "boy")] - metric_total = pivoted.loc[(total_label(),), (metrics[0], "Subtotal")] + metric_total = pivoted.loc[(total,), (metrics[0], "Subtotal")] # Includes the grand-total corner: all-blank sums must not revive it as 0. assert mixed.isna().all() assert pd.notna(leaf) assert pd.notna(metric_total) if mode is None: assert leaf == 10 - assert metric_total == (19 if with_rollups else 80) + assert metric_total == ( + 19 if with_rollups else (80 if aggregation == "Sum" else 20) + ) def test_pivot_df_show_values_as_with_combined_metrics(): @@ -4705,3 +4710,65 @@ def test_apply_client_processing_csv_format_pivot_table_multiple_rows(): def test_pivot_rollup_reducer_preserves_blank_and_zero(reducer: str): assert pd.isna(_reduce(pd.Series([float("nan"), float("nan")]), reducer)) assert _reduce(pd.Series([0, float("nan")]), reducer) == 0 + + +@pytest.mark.parametrize("layout", ["COLUMNS", "ROWS"]) +@pytest.mark.parametrize("combine", [False, True]) +@pytest.mark.parametrize("transpose", [False, True]) +def test_pivot_mixed_totals_preserve_integer_csv(layout, combine, transpose): + """Blank totals must not round integer leaves or change their CSV spelling.""" + large = 2**53 + 1 + frame = pd.DataFrame( + { + "nation": ["US", "UK"], + "gender": ["boy", "boy"], + "SUM(a)": [large, 2], + "SUM(b)": [3, 4], + } + ) + result = pivot_table_v2( + frame, + { + "groupbyRows": ["nation"], + "groupbyColumns": ["gender"], + "metrics": ["SUM(a)", "SUM(b)"], + "metricsLayout": layout, + "combineMetric": combine, + "transposePivot": transpose, + "rowTotals": True, + "colTotals": True, + }, + apply_number_format=False, + ) + csv = result.to_csv() + assert str(large) in csv + assert str(large - 1) not in csv + assert ".0" not in csv + + +@pytest.mark.parametrize("mode", [None, "percent_row", "percent_col", "percent_total"]) +@pytest.mark.parametrize("transpose", [False, True]) +@pytest.mark.parametrize("single_metric", [False, True]) +def test_combined_metric_subtotals(mode, transpose, single_metric): + frame = show_values_as_df() + frame["MAX(num)"] = [1, 3, 6, 10] + metrics = ["SUM(num)"] if single_metric else ["SUM(num)", "MAX(num)"] + result = pivot_table_v2( + frame, + { + "groupbyRows": ["nation"], + "groupbyColumns": ["gender"], + "metrics": metrics, + "combineMetric": True, + "transposePivot": transpose, + "rowTotals": True, + "colTotals": True, + "showValuesAs": mode, + }, + apply_number_format=False, + ) + # Transpose swaps the dimension axes before constructing the pivot. + groups = ["US", "UK"] if transpose else ["boy", "girl"] + for group in groups: + subtotal = result.loc[:, (group, "Subtotal")] + assert subtotal.notna().all() if single_metric else subtotal.isna().all() From c7f29d26ae6f3941df203bacb7e58025fd978201 Mon Sep 17 00:00:00 2001 From: Matt Fitzgerald Date: Sun, 18 Oct 2026 10:20:00 +1100 Subject: [PATCH 5/7] fix(pivot-table): finish mixed-total review follow-ups --- superset/charts/client_processing.py | 6 ++--- .../charts/test_client_processing.py | 25 ++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/superset/charts/client_processing.py b/superset/charts/client_processing.py index f5a965cb89b3..54d09a098728 100644 --- a/superset/charts/client_processing.py +++ b/superset/charts/client_processing.py @@ -169,8 +169,8 @@ def _collapsed_metric(present: list[Any] | set[Any]) -> Any: ``react-pivottable/utilities.ts``); mirror it here so exported/scheduled values agree with what the chart shows. """ - distinct = set(present) - return distinct.pop() if len(distinct) == 1 else None + distinct = present if isinstance(present, set) else set(present) + return next(iter(distinct)) if len(distinct) == 1 else None def _broadcast(total: pd.Series, block: pd.DataFrame, axis: int) -> pd.DataFrame: @@ -625,7 +625,7 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: Returns `(None, reducer)` when `block` spans more than one distinct metric: such a total has no single meaningful value, so the caller blanks it instead of summing across metrics that share no unit - (mirrors the renderer, see #44657). + (matches the browser change proposed in #44657). """ present = [ _metric_of_column(column, totals_metric_level) for column in block.columns diff --git a/tests/unit_tests/charts/test_client_processing.py b/tests/unit_tests/charts/test_client_processing.py index a94fcfdadb54..8509c9a58317 100644 --- a/tests/unit_tests/charts/test_client_processing.py +++ b/tests/unit_tests/charts/test_client_processing.py @@ -25,6 +25,7 @@ from sqlalchemy.orm.session import Session from superset.charts.client_processing import ( + _collapsed_metric, _reduce, apply_client_processing, apply_pivot_number_formats, @@ -4715,7 +4716,9 @@ def test_pivot_rollup_reducer_preserves_blank_and_zero(reducer: str): @pytest.mark.parametrize("layout", ["COLUMNS", "ROWS"]) @pytest.mark.parametrize("combine", [False, True]) @pytest.mark.parametrize("transpose", [False, True]) -def test_pivot_mixed_totals_preserve_integer_csv(layout, combine, transpose): +def test_pivot_mixed_totals_preserve_integer_csv( + layout: str, combine: bool, transpose: bool +): """Blank totals must not round integer leaves or change their CSV spelling.""" large = 2**53 + 1 frame = pd.DataFrame( @@ -4749,7 +4752,10 @@ def test_pivot_mixed_totals_preserve_integer_csv(layout, combine, transpose): @pytest.mark.parametrize("mode", [None, "percent_row", "percent_col", "percent_total"]) @pytest.mark.parametrize("transpose", [False, True]) @pytest.mark.parametrize("single_metric", [False, True]) -def test_combined_metric_subtotals(mode, transpose, single_metric): +@pytest.mark.parametrize("layout", ["COLUMNS", "ROWS"]) +def test_combined_metric_subtotals( + mode: str | None, transpose: bool, single_metric: bool, layout: str +): frame = show_values_as_df() frame["MAX(num)"] = [1, 3, 6, 10] metrics = ["SUM(num)"] if single_metric else ["SUM(num)", "MAX(num)"] @@ -4759,6 +4765,7 @@ def test_combined_metric_subtotals(mode, transpose, single_metric): "groupbyRows": ["nation"], "groupbyColumns": ["gender"], "metrics": metrics, + "metricsLayout": layout, "combineMetric": True, "transposePivot": transpose, "rowTotals": True, @@ -4768,7 +4775,19 @@ def test_combined_metric_subtotals(mode, transpose, single_metric): apply_number_format=False, ) # Transpose swaps the dimension axes before constructing the pivot. - groups = ["US", "UK"] if transpose else ["boy", "girl"] + metrics_on_rows = layout == "ROWS" + if metrics_on_rows: + result = result.T + groups = ["US", "UK"] if transpose != metrics_on_rows else ["boy", "girl"] for group in groups: subtotal = result.loc[:, (group, "Subtotal")] assert subtotal.notna().all() if single_metric else subtotal.isna().all() + + +@pytest.mark.parametrize("metrics", [set(), {"SUM(num)"}, {"SUM(num)", "MAX(num)"}]) +def test_collapsed_metric_preserves_shared_metric_set(metrics: set[str]): + original = metrics.copy() + expected = "SUM(num)" if len(metrics) == 1 else None + assert _collapsed_metric(metrics) == expected + assert _collapsed_metric(metrics) == expected + assert metrics == original From 044e2a578ef505454802bbcb3ff865f8cff16c1f Mon Sep 17 00:00:00 2001 From: Matt Fitzgerald Date: Tue, 20 Oct 2026 16:32:35 +1100 Subject: [PATCH 6/7] fix: preserve exact pivot integers in XLSX with blank totals --- superset/charts/client_processing.py | 4 +- superset/utils/excel.py | 7 ++- .../charts/test_client_processing.py | 43 +++++++++++++++++++ tests/unit_tests/utils/excel_tests.py | 13 ++++++ 4 files changed, 65 insertions(+), 2 deletions(-) diff --git a/superset/charts/client_processing.py b/superset/charts/client_processing.py index 3ba0fac5d679..d8419e390a5d 100644 --- a/superset/charts/client_processing.py +++ b/superset/charts/client_processing.py @@ -208,7 +208,7 @@ def _reduce( ) -> Any: """Apply a rollup reducer (``sum``/``min``/``max``), skipping empty cells. - A percentage-mode rollup with no non-null observations stays blank: + A rollup with no non-null observations stays blank: ``sum`` otherwise defaults an all-NaN input to 0, treating missing values as a measured zero. """ @@ -705,6 +705,8 @@ def collapse(block: pd.DataFrame) -> tuple[Optional[pd.DataFrame], str]: ) raise + # These labels were inserted above; no columns are removed + # before this point. Keep strict lookup to catch broken invariants. subtotal_values = df.iloc[slice_, :].drop(columns=mixed_metric_totals) if aggfunc != CURRENCY_CONTEXT_AGGREGATION: subtotal_values = subtotal_values.apply( diff --git a/superset/utils/excel.py b/superset/utils/excel.py index 269ace9d725c..437cd6739dd4 100644 --- a/superset/utils/excel.py +++ b/superset/utils/excel.py @@ -219,7 +219,12 @@ def apply_column_types( series = df.iloc[:, idx] if column_type == GenericDataType.NUMERIC: try: - series = pd.to_numeric(series) + # Nullable integers retain exact values alongside blank totals. + # Convert to object before apply, which would otherwise coerce + # nullable integers through float64 during scalar mapping. + series = pd.to_numeric(series, dtype_backend="numpy_nullable").astype( + object + ) # if the number is too large, convert it to a string # Excel does not support numbers larger than 10^15 series = series.apply( diff --git a/tests/unit_tests/charts/test_client_processing.py b/tests/unit_tests/charts/test_client_processing.py index d8ff0cb92642..c59d81000953 100644 --- a/tests/unit_tests/charts/test_client_processing.py +++ b/tests/unit_tests/charts/test_client_processing.py @@ -5107,3 +5107,46 @@ def test_collapsed_metric_preserves_shared_metric_set(metrics: set[str]): assert _collapsed_metric(metrics) == expected assert _collapsed_metric(metrics) == expected assert metrics == original + + +@pytest.mark.parametrize("combine", [False, True]) +@pytest.mark.parametrize("transpose", [False, True]) +def test_pivot_mixed_totals_preserve_integer_xlsx(combine: bool, transpose: bool): + """The full XLSX path keeps large integers as exact text and small ones numeric.""" + from openpyxl import load_workbook + + large = 2**53 + 1 + source = pd.DataFrame( + { + "nation": ["US", "UK"], + "gender": ["boy", "boy"], + "SUM(a)": [str(large), "2"], + "SUM(b)": [3, 4], + } + ) + result = apply_client_processing( + { + "queries": [ + { + "result_format": ChartDataResultFormat.XLSX, + "data": excel.df_to_excel(source, index=False), + } + ] + }, + { + "viz_type": "pivot_table_v2", + "groupbyRows": ["nation"], + "groupbyColumns": ["gender"], + "metrics": ["SUM(a)", "SUM(b)"], + "metricsLayout": "ROWS", + "combineMetric": combine, + "transposePivot": transpose, + "rowTotals": True, + "colTotals": True, + }, + ) + sheet = load_workbook(BytesIO(result["queries"][0]["data"])).active + cells = [cell for row in sheet for cell in row] + assert any(cell.value == str(large) and cell.data_type == "s" for cell in cells) + assert any(cell.value == 3 and cell.data_type == "n" for cell in cells) + assert not any(str(cell.value).startswith(str(large - 1)) for cell in cells) diff --git a/tests/unit_tests/utils/excel_tests.py b/tests/unit_tests/utils/excel_tests.py index 560c62da3a7a..719d07dbd68a 100644 --- a/tests/unit_tests/utils/excel_tests.py +++ b/tests/unit_tests/utils/excel_tests.py @@ -297,3 +297,16 @@ def test_column_data_types_with_large_numeric_values(): "1100108628127863", "18014398509481984", ] + + +def test_numeric_column_with_blanks_preserves_large_integers() -> None: + """Blank cells must not round integers before Excel's text conversion.""" + large = 2**53 + 1 + df = pd.DataFrame({"value": pd.Series([large, None, 3], dtype=object)}) + apply_column_types(df, [GenericDataType.NUMERIC]) + sheet = load_workbook(io.BytesIO(df_to_excel(df, index=False))).active + assert sheet["A2"].value == str(large) + assert sheet["A2"].data_type == "s" + assert sheet["A3"].value is None + assert sheet["A4"].value == 3 + assert sheet["A4"].data_type == "n" From 4f9a8cbc90cd555020cf87fe59edb4a870271e1c Mon Sep 17 00:00:00 2001 From: Matt Fitzgerald Date: Tue, 20 Oct 2026 17:05:31 +1100 Subject: [PATCH 7/7] test(mcp): refresh native tool inventory for PNG previews --- .../mcp_service/native_tool_inventory.json | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/unit_tests/mcp_service/native_tool_inventory.json b/tests/unit_tests/mcp_service/native_tool_inventory.json index 5c65bcf4c468..aa6a827e947c 100644 --- a/tests/unit_tests/mcp_service/native_tool_inventory.json +++ b/tests/unit_tests/mcp_service/native_tool_inventory.json @@ -234,11 +234,11 @@ "compact_description_chars": 250, "description_chars": 3577, "entry_bytes": { - "structured": 25915, + "structured": 26411, "text_only": 6037 }, "input_schema_bytes": 2037, - "output_schema_bytes": 19862 + "output_schema_bytes": 20358 }, "generate_dashboard": { "annotations": { @@ -343,14 +343,14 @@ "openWorldHint": false, "readOnlyHint": true }, - "compact_description_chars": 241, - "description_chars": 434, + "compact_description_chars": 240, + "description_chars": 553, "entry_bytes": { - "structured": 8610, - "text_only": 3359 + "structured": 9342, + "text_only": 3595 }, - "input_schema_bytes": 2679, - "output_schema_bytes": 5235 + "input_schema_bytes": 2793, + "output_schema_bytes": 5731 }, "get_chart_sql": { "annotations": { @@ -1194,11 +1194,11 @@ "compact_description_chars": 45, "description_chars": 2416, "entry_bytes": { - "structured": 26567, + "structured": 27063, "text_only": 6689 }, "input_schema_bytes": 3913, - "output_schema_bytes": 19862 + "output_schema_bytes": 20358 }, "update_chart_preview": { "annotations": { @@ -1267,16 +1267,16 @@ }, "totals": { "catalog_bytes": { - "structured": 522735, - "text_only": 214220 + "structured": 524459, + "text_only": 214456 }, "compact_catalog_bytes": { - "structured": 470159, - "text_only": 161644 + "structured": 471762, + "text_only": 161759 }, "largest_entry": { "structured": { - "bytes": 26567, + "bytes": 27063, "name": "update_chart" }, "text_only": {