sadpandajoe commented on code in PR #43636:
URL: https://github.com/apache/superset/pull/43636#discussion_r3886177756


##########
docs/docs/using-superset/exploring-data.mdx:
##########
@@ -151,6 +151,19 @@ see some data!
 You should see months in the rows and Department and Travel Class in the 
columns. Publish this chart
 to your existing Tutorial Dashboard you created earlier.
 
+:::note
+Row and column totals/subtotals for the Pivot Table are correct even for 
non-additive metrics,
+such as ratios (`SUM(a)/SUM(b)`), `COUNT_DISTINCT`, `AVG`, and percentiles, 
not just additive ones
+like `SUM` or `COUNT`. Additive metrics derive their totals client-side, by 
reducing the same

Review Comment:
   This says additive metrics take the client-side path, but 
`allMetricsAdditive` enables that path only when every selected metric is 
additive. A Pivot Table with `SUM` and `AVG` therefore recomputes the `SUM` in 
the database too. Could this qualify the fast path as applying only when all 
selected metrics are additive?



##########
docs/docs/using-superset/exploring-data.mdx:
##########
@@ -151,6 +151,19 @@ see some data!
 You should see months in the rows and Department and Travel Class in the 
columns. Publish this chart
 to your existing Tutorial Dashboard you created earlier.
 
+:::note
+Row and column totals/subtotals for the Pivot Table are correct even for 
non-additive metrics,
+such as ratios (`SUM(a)/SUM(b)`), `COUNT_DISTINCT`, `AVG`, and percentiles, 
not just additive ones
+like `SUM` or `COUNT`. Additive metrics derive their totals client-side, by 
reducing the same
+full-detail query results used to build the table. Non-additive metrics can't 
be combined that
+way, so Superset instead issues a database query at each total's own 
granularity, so the total
+reflects the metric's definition evaluated at that level rather than an 
incorrect combination of
+the displayed cells. Because of this, there's no separate "Aggregation 
function" control for
+totals in the Pivot Table: a total always reflects the metric's own 
definition. The Table chart's
+**Show summary** row is different: its **Summary aggregation** control can 
override each metric's

Review Comment:
   This promises the Summary aggregation control can override each metric, but 
it only overrides simple metrics; custom-SQL and saved metrics retain their own 
definitions. A Table containing a ratio metric will not behave as described. 
Could this scope the override to simple metrics?



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