rusackas opened a new issue, #41463: URL: https://github.com/apache/superset/issues/41463
*Please make sure you are familiar with the SIP process documented* [here](https://github.com/apache/superset/issues/5602). The SIP will be numbered by a committer upon acceptance. > **Status:** Draft for DISCUSS. A working proof-of-concept implementing this proposal — across both the Table and Pivot Table charts, with the performance work below — lives in **#41184** and is referenced throughout. Every claim marked "verified" was confirmed against a running stack (Postgres + the example `birth_names` dataset). ## [SIP] Proposal for correct totals and subtotals for non-additive metrics in Table and Pivot Table charts ### Motivation Superset computes the totals and subtotals shown in Table and Pivot Table charts by **re-aggregating values that have already been aggregated** (or by running a total query that drops the metric's post-processing). This is correct only for *additive* metrics (`SUM`, `COUNT`, `MIN`, `MAX`). For any **non-additive** metric the result is mathematically wrong: - A ratio metric `SUM(actual) / SUM(target)` shows a total equal to the **sum of the per-row ratios** instead of `SUM(all actual) / SUM(all target)` (e.g. a "completion %" grand total of 100%+ instead of 11%). - `COUNT(DISTINCT user)` shows a total equal to the **sum of per-group distinct counts**, double-counting anything appearing in more than one group. - `AVG`, `MEDIAN`, `PERCENTILE`, `STDDEV` are summed, which is meaningless. - The Table chart's "Percentage metrics" / contribution columns are dropped from the summary row entirely (come back empty/zero). This is one of the longest-standing and most-reported correctness gaps in the charting layer, and it blocks migrations from Tableau / Power BI / Qlik / Excel, all of which get this right. It spans **both** the Pivot Table and the regular Table chart, which today use two different (and separately broken) total mechanisms. **This SIP consolidates the following reported issues** (tracked under #25747 as the canonical thread): - #25747 — pivot totals wrong for non-additive (ratio) metrics *(canonical)* - #32260 — completion-percentage subtotal **and** grand total wrong (multi-level rows) - #38674 — pivot Grand Total sums percentages instead of recomputing the ratio - #36165 — Table `COUNT_DISTINCT` summary value mismatch - #37627 — Table "Percentage metrics" column empty/zero when "Show summary" is on (still repros on 6.0) - #34350 / #34425 / #34426 — Table percentage totals wrong / page-scoped Design discussion: #29297 ("Totals in Table Charts"). Out of scope: #39223 (a separate v6 `DISTINCT_AVG`/`DISTINCT_SUM` SQL-generation regression). ### Proposed Change **Core principle:** totals and subtotals must be computed by the database at the grouping granularity they are displayed for, never derived on the client by re-aggregating already-aggregated cells. The key insight that keeps this tractable: *a SQL-aggregate metric is correct at any grouping level if the same expression is evaluated grouped at that level* — so no formula language or per-metric "non-additive flag" is required; we simply stop summing cells and ask the database for the total/subtotal rows. **Pivot Table.** Replace the client-side `pivot_table(margins=True)` / react-pivottable aggregation with database-computed rollup levels. Each rollup level is one prefix of the row dimensions crossed with one prefix of the column dimensions (the empty combination is the grand total). The pivot emits a single query carrying a `grouping_sets` list of these levels; the database computes every level, and the frontend places each pre-computed value into its cell/subtotal/total slot (the react-pivottable aggregator becomes a passthrough). *(Verified in-app: a `rows=[gender, state]` ratio pivot now shows the correct 11% subtotal/grand total instead of summed-cell values.)* **Table chart.** The grand-total/summary row is already a separate no-GROUP-BY query, so SQL-aggregate totals are already correct; the only defect was that the summary query dropped post-processing, so percent/contribution columns came back empty. The fix retains post-processing on that query. *(Verified in-app.)* **Performance.** Three layered optimizations keep this from regressing query cost: 1. **Additive fast-path** — when every metric is additive, a single full-detail query is issued and the rollup levels are synthesized on the client (sum/min/max), preserving today's behavior and cost for additive pivots. 2. **`GROUPING SETS` single query** — for non-additive metrics, all rollup levels are computed in one scan using native `GROUPING SETS` + `GROUPING()` markers, gated by a new `supports_grouping_sets` engine capability. *(Verified in-app on Postgres: one query returns all levels.)* 3. **Per-level fallback** — engines without native support transparently expand the `grouping_sets` into one query per displayed level (only the enabled totals/subtotals are queried). Same results, more scans. ### New or Changed Public Interfaces - **Query object / chart-data API:** a new optional `grouping_sets` field (list of column-name lists) on the query object and `ChartDataQueryObjectSchema`. When unset, query generation is byte-identical to today. - **DB engine spec:** a new `supports_grouping_sets` capability flag (default `False`; enabled for Postgres, BigQuery, Snowflake, Presto/Trino). - **Pivot Table plugin:** the per-table **"Aggregation function"** control is removed (totals now reflect each metric's own definition); saved charts that set it simply ignore it. React components consume per-level `QueryData[]`. - No new or changed REST endpoints beyond the additive query-object field. ### New dependencies None. `GROUPING SETS` / `GROUPING()` are standard SQL emitted through the existing SQLAlchemy/engine-spec layer; no new npm or PyPI packages. ### Migration Plan and Compatibility - No database (metadata) migration required. - Behavioral change: non-additive totals/subtotals change from (wrong) sums to correct values — documented in `UPDATING.md`, along with the removal of the pivot "Aggregation function" control. - Additive-only charts are unaffected (fast-path); engines without `GROUPING SETS` use the per-level fallback with identical results. - A feature flag can gate the rollout if the community prefers an opt-in period. ### Rejected Alternatives - **A formula-aware engine / metric mini-language.** Unnecessary for SQL-aggregate metrics (evaluating the same expression at the target grouping is exact) and fragile for arbitrary user SQL. - **One query per rollup level as the *primary* mechanism** (as prototyped in #34592). Correct but `O(2^(R+C))` queries; kept only as the fallback where `GROUPING SETS` is unavailable. This SIP builds on #34592's approach and salvages its rollup-combination generator. - **Smarter client-side re-aggregation** (weighted sums, etc.). Cannot recover information destroyed by the first aggregation (e.g. distinct counts). - **A new rendering engine (AntV S2, SIP-205 / #38586).** Routes around the problem but still needs correct data from the backend; orthogonal and parked behind the Extensions project. - **Cosmetic relabeling only** (rename Total→Summary, the resolution previously reached in #29297). Useful for clarity but does not fix the numbers; complementary, not a substitute. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
