rusackas opened a new pull request, #43027:
URL: https://github.com/apache/superset/pull/43027

   ### SUMMARY
   Today the "Show summary" (`show_totals`) row in the Table chart and AG Grid 
Table chart always uses whatever aggregation the underlying metric already 
specifies (e.g. a `SUM(x)` metric totals as a sum). A user displaying an 
`AVG`/rate/score metric per row has no way to get a meaningful summary — they'd 
have to add a second, differently-aggregated metric and hide/relabel it, which 
doesn't work at all for Aggregate-mode metrics that are already pre-aggregated.
   
   This adds a `totals_aggregate` control (Sum / Average, default Sum, only 
visible when "Show summary" is on) that lets the user pick the totals row's 
aggregation independently of each metric's own aggregation. It's implemented 
via a new shared `getTotalsMetrics` utility (`@superset-ui/chart-controls`) 
that clones each **Simple (adhoc)** metric with its `aggregate` swapped, used 
by both plugins' totals query construction.
   
   This is safe because the "Show summary" row is produced by a **separate 
query with no `GROUP BY`** (`columns: []`) — the database evaluates each metric 
fresh over all rows, so swapping the aggregate for just that query is a 
correct, independent computation, not a re-aggregation of already-aggregated 
per-row values (the class of bug 
[SIP-216](https://github.com/apache/superset/issues/41463) fixed for Pivot 
Table subtotals via `GROUPING SETS`).
   
   No backend changes are required: `show_totals` has always been a 
frontend-only form-data key, and `ChartDataAdhocMetricSchema` already accepts 
`AVG` for any Simple metric on any query object.
   
   **Known limitations (out of scope for this PR):**
   - **Median is not supported.** There's no universal SQL `MEDIAN` — Postgres 
needs `PERCENTILE_CONT(0.5) WITHIN GROUP (...)`, MySQL <8 lacks it entirely — 
so it needs new backend aggregate-function support. Will be filed as a 
follow-up issue linked from here.
   - **Custom-SQL and saved (string) metrics keep their own native aggregate** 
in the totals row; the Sum/Average override only applies to Simple (adhoc) 
metrics, since there's no safe way to rewrite an arbitrary SQL expression's 
aggregate function without parsing it.
   
   Fixes #43021
   Related: #41463 / #41184 (SIP-216 — same underlying "totals row aggregation" 
concern, but for making totals *correct* for whatever aggregation a metric 
already uses, on Pivot Table only)
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A — this PR was implemented and verified via automated tests only (see 
Testing instructions); no interactive browser session was available to capture 
screenshots.
   
   ### TESTING INSTRUCTIONS
   1. Create/open a Table (or AG Grid Table) chart in Aggregate mode with a 
`SUM`-aggregated Simple metric, group by a dimension.
   2. Enable **Show summary** — note the new **Summary aggregation** dropdown 
appears underneath, defaulted to "Sum".
   3. Confirm the summary row shows the sum of the metric (unchanged behavior).
   4. Switch **Summary aggregation** to "Average" and re-run the query. Confirm 
the summary row now shows the average of the metric across all underlying rows 
(verify against a manual `AVG()` query) — not the sum, and not a naive average 
of the displayed per-row values.
   5. Repeat for AG Grid Table in both Aggregate mode and Raw/records query 
mode (the summary columns primed via `rawSummaryColumns`).
   6. Add a custom-SQL metric or a saved metric alongside a Simple metric; 
confirm its totals-row value is unaffected by the Summary aggregation setting 
(it keeps using its own native aggregation).
   
   Automated coverage added:
   - `plugin-chart-table/test/buildQuery.test.ts` — new `Totals Aggregation` 
describe block (default SUM, AVG override, SQL/saved metrics pass through 
unchanged).
   - `plugin-chart-ag-grid-table/test/buildQuery.test.ts` — new cases for 
aggregate-mode and raw-mode totals aggregate override.
   - `tests/integration_tests/non_additive_totals_tests.py` — new 
`TestTableTotalsAggregateOverride`, an end-to-end guard (against the real 
`birth_names` fixture) that an AVG-override totals query returns a true `SUM / 
COUNT` row-level average, not the metric's own SUM.
   
   All frontend unit tests for the two plugin packages pass locally (456/456), 
`tsc --build` is clean, and `pre-commit run` is green on the changed files. The 
new backend integration test could not be run in this environment (pre-existing 
local test-DB migration issue unrelated to this change — reproduced with 
pre-existing tests in the same file) and should be verified in CI.
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: #43021
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API


-- 
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]

Reply via email to