sadpandajoe commented on code in PR #42594:
URL: https://github.com/apache/superset/pull/42594#discussion_r3740966960
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -434,7 +434,14 @@ export function extractDataTotalValues(
return prev;
}
const value = datum[curr] || 0;
- return prev + (value as number);
+ // Query results with integers beyond Number.MAX_SAFE_INTEGER are
+ // parsed as native BigInt (see
+ // packages/superset-ui-core/src/connection/callApi/parseResponse.ts).
+ // Normalize to Number before summing so BigInt and Number values
+ // can be combined without throwing (see #36401).
+ const numericValue =
+ typeof value === 'bigint' ? Number(value) : (value as number);
Review Comment:
Normalization here only covers stacked totals, but the transform then calls
`sortAndFilterSeries` with the default `SortSeriesType.Sum`. In the issue's
two-row `a` series, one value is parsed as `bigint` and the other as `number`,
so Lodash's `sumBy` still throws before `extractSeries` reaches the new Expand
conversion; could we normalize metric values before the default series-sort
aggregation and cover that two-row path?
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -699,7 +706,17 @@ export function extractSeries(
stack === StackControlsValue.Expand &&
totalStackedValue !== undefined
) {
- value = ((value || 0) as number) / totalStackedValue;
+ // Query results with integers beyond Number.MAX_SAFE_INTEGER are
+ // parsed as native BigInt (see
+ //
packages/superset-ui-core/src/connection/callApi/parseResponse.ts).
+ // totalStackedValue is always a Number (extractDataTotalValues
+ // normalizes it), so dividing a raw BigInt datum value by it
+ // throws; normalize to Number first (see #36401).
+ const numericValue =
Review Comment:
The per-datum conversion is limited to `Expand`, so `Stream` still emits the
raw `bigint`; `getBaselineSeriesForStream` then evaluates `0.5 * delta` and
throws before ECharts renders. Could we normalize stacked metric values before
the mode-specific branch and add a Stream transform case that asserts numeric
series output?
--
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]
