bito-code-review[bot] commented on code in PR #43636: URL: https://github.com/apache/superset/pull/43636#discussion_r3877838601
########## docs/docs/using-superset/exploring-data.mdx: ########## @@ -151,6 +151,16 @@ 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 computed by the database, at the same +granularity as the total itself, rather than by summing the already-aggregated cells shown in the Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Misleading totals computation claim</b></div> <div id="fix"> The note states totals are "computed by the database," but `plugin-chart-pivot-table/src/plugin/buildQuery.ts` (lines 82-99) shows additive metrics (SUM/COUNT/MIN/MAX) use a single full-detail query with client-side reduction via `synthesizeAdditiveLevels` in `utilities.ts`. Only non-additive metrics use `GROUPING SETS` for DB-side computation. Reword to distinguish the two paths. </div> </div> <small><i>Code Review Run #a00085</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them ########## docs/docs/using-superset/exploring-data.mdx: ########## @@ -151,6 +151,16 @@ 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 computed by the database, at the same +granularity as the total itself, rather than by summing the already-aggregated cells shown in the +table. This means totals are correct for non-additive metrics too, such as ratios +(`SUM(a)/SUM(b)`), `COUNT_DISTINCT`, `AVG`, and percentiles, not just additive ones like `SUM` or +`COUNT`. Because of this, there's no separate "Aggregation function" control for totals: a total +always reflects the metric's own definition evaluated at that total's level. The Table chart's +**Show summary** row behaves the same way for percentage metrics. Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Incorrect Table chart claim</b></div> <div id="fix"> The note claims the Table chart's Show summary row "behaves the same way," but `plugin-chart-table/src/controlPanel.tsx` (lines 472-498) defines a `totals_aggregate` control labeled "Summary aggregation" with ORIGINAL/SUM/AVG choices that explicitly overrides the metric's own aggregation for the summary row. This contradicts the note's claim that there's no separate aggregation control. Remove or correct this sentence. </div> </div> <small><i>Code Review Run #a00085</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them -- 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]
