Copilot commented on code in PR #43684:
URL: https://github.com/apache/superset/pull/43684#discussion_r3887879596
##########
superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/transformProps.ts:
##########
@@ -117,32 +122,77 @@ export default function transformProps(
...formData,
};
- const groupbyColumn = ensureIsArray(groupby)[0];
- const categoryLabel = getColumnLabel(groupbyColumn);
const leftMetricLabel = leftMetric ? getMetricLabel(leftMetric) : '';
const rightMetricLabel = rightMetric ? getMetricLabel(rightMetric) : '';
const leftSeriesName = leftLabel || leftMetricLabel;
const rightSeriesName = rightLabel || rightMetricLabel;
+ const coltypeMapping = getColtypesMapping(queriesData[0]);
+ const groupbyColumns = ensureIsArray(groupby);
+ const groupbyLabels = groupbyColumns.map(getColumnLabel);
+
const defaultFormatter = currencyFormat?.symbol
? new CurrencyFormatter({ d3Format: xAxisFormat, currency: currencyFormat
})
: getNumberFormatter(xAxisFormat);
- const categories = data.map(row => formatCategory(row[categoryLabel]));
- const leftData = data.map(row => {
- const value = Number(row[leftMetricLabel] ?? 0);
- return {
- value: -Math.abs(value),
- label: LABEL_LEFT,
- };
- });
- const rightData = data.map(row => {
- const value = Number(row[rightMetricLabel] ?? 0);
- return {
- value: Math.abs(value),
- label: LABEL_RIGHT,
- };
+ const categories = data.map(datum =>
+ extractGroupbyLabel({ datum, groupby: groupbyLabels, coltypeMapping }),
+ );
+ const categoryKeys = data.map((datum, index) => {
+ const label = categories.at(index) ?? '';
+ return `${label}__${JSON.stringify(
+ groupbyLabels.map(col =>
+ Object.hasOwn(datum, col) ? datum[col] : undefined,
+ ),
+ )}`;
});
+ const categoryByKey = new Map(
+ categoryKeys.flatMap((key, index) => {
+ const label = categories.at(index);
+ return label === undefined ? [] : [[key, label] as const];
+ }),
+ );
+
+ const labelMap = data.reduce<Record<string, string[]>>(
+ (acc, datum, index) => {
+ const uniqueKey = categoryKeys.at(index);
+ if (uniqueKey === undefined) {
+ return acc;
+ }
+ return {
+ ...acc,
+ [uniqueKey]: groupbyLabels.map(col =>
+ Object.hasOwn(datum, col) ? (datum[col] as string) : '',
+ ),
+ };
Review Comment:
Building `labelMap` with `{ ...acc }` copies every previously added entry
for every row, making this transform quadratic. The chart's row-limit control
defaults to 10,000, so this can perform roughly 50 million property copies on
each render; update the accumulator in place (or build entries once) to keep
this linear.
--
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]
