sadpandajoe commented on code in PR #44657:
URL: https://github.com/apache/superset/pull/44657#discussion_r4179531111
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts:
##########
@@ -482,10 +548,28 @@ const baseAggregatorTemplates = {
sum: 0 as any,
currencySet: new Set<string>(),
push(record: PivotRecord) {
- if (Number.isNaN(Number(record[attr]))) {
- this.sum = record[attr];
+ const val = record[attr];
+ // A metric's own value can be a real SQL NULL (e.g. SUM over an
+ // empty group). Number(null) coerces to 0, not NaN, so it would
+ // otherwise fall into the numeric branch below, where
+ // parseFloat(String(null)) ('parseFloat("null")') is NaN and
+ // silently poisons the running sum -- skip it entirely, the same
+ // way a group with no matching leaf record at all is excluded.
+ if (val === null || val === undefined) {
Review Comment:
With blue=SQL NULL and red=30, `Sum as Fraction of Total` renders blue as
`0.0%` instead of blank: skipping the only input leaves `sum` at zero, so the
numerator’s null guard no longer applies. Could all-null scopes retain null
while mixed scopes still ignore null inputs?
--
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]
