EnxDev commented on PR #41754:
URL: https://github.com/apache/superset/pull/41754#issuecomment-4894351315
@amaannawab923 all three addressed (commits e0290e75d3 / 4a961d5344, rebased
onto the master merge; 256/256 plugin tests passing).
**Empty `{}` totals**
([thread](https://github.com/apache/superset/pull/41754#discussion_r3529843821))
— confirmed, and there is a concrete path to it: `processComparisonTotals`
returns `{}` when the time-comparison totals result has no rows, which slipped
past `totals !== undefined` and pinned a blank row. The gate is now `showTotals
&& totals !== undefined && Object.keys(totals).length > 0`, with a RED-first
test (`totals = {}` → no pinned row). Note the SUM-over-zero-rows case still
pins as intended: the backend returns a row with NULL values there, so the keys
are present.
**Calculated-column coverage**
([thread](https://github.com/apache/superset/pull/41754#discussion_r3529843829))
— test added: a calculated column name in `all_columns` + `rawSummaryColumns`
flows through the intersection into the SUM metrics. To be precise about what
it does and doesn't prove: a frontend unit test can't exercise backend
resolution — from `buildQuery`'s perspective a calculated column is just
another string. The backend side is the standard SIMPLE adhoc-metric path
(column name resolved through the dataset's columns, expression substituted
server-side), which is the same mechanism Explore's own adhoc metrics use on
calculated columns. What the new test genuinely pins is that the selection
intersection stays name-based and never silently starts assuming physical
columns only.
**Extra round trips**
([thread](https://github.com/apache/superset/pull/41754#discussion_r3529843835))
— yes, conscious call, with these bounds: a saved *aggregate* + summary chart
does zero extra work (totals ride the initial formData-driven query; the nudge
only fires in the renderTrigger gap). A saved *raw* + summary chart pays one
priming re-query on first load — the base query is a server-side cache hit,
only the totals query is new work. The toggle-off reset query is the deliberate
trade-off: the alternative (clearing the flag when data arrives) would fire an
extra query after *every* totals fetch, which is strictly worse; toggle-off
stays visually instant regardless because the grid gate hides the row before
the query returns.
--
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]
