rusackas commented on code in PR #42810:
URL: https://github.com/apache/superset/pull/42810#discussion_r3723715901
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts:
##########
@@ -805,7 +824,10 @@ const baseAggregatorTemplates = {
}
}
const denominatorAggregator = data.getAggregator(selRow, selCol);
- if (!denominatorAggregator.inner) {
+ if (
+ !denominatorAggregator.inner ||
+ denominatorAggregator.metricCollision
Review Comment:
Fair catch, though as it happened this got resolved by dropping the
collision-tracking approach entirely rather than typing around it. Rebased onto
a follow-up fix pushed to the base branch that keeps `metricAxis` in sync with
`inner` on every push instead, so there's no extra property on the aggregator
to type.
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts:
##########
@@ -814,7 +836,16 @@ const baseAggregatorTemplates = {
return acc;
}
- return this.inner.value() / acc;
+ // A DB-computed rollup value can legitimately be a real SQL NULL
+ // (e.g. AVG over an empty group). `null / acc` coerces to `0` in
+ // JS, which would render a measured "0.0%" for a value that
+ // should stay blank, same as it does in "Actual values" mode.
+ const numerator = this.inner.value();
+ if (numerator === null) {
+ return null;
+ }
+
+ return numerator / acc;
},
Review Comment:
Confirmed, this was a real gap independent of the numerator one. Added an
`acc === null` guard before the numerator check. The rendered text already came
out blank either way (the shared formatter blanks non-finite values), but
`value()` itself was returning Infinity instead of null, which matters for
anything reading it directly like value-based sorting. Added a unit test on
`value()` itself since the DOM-level test alone couldn't tell the difference.
--
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]
