sadpandajoe commented on code in PR #44550:
URL: https://github.com/apache/superset/pull/44550#discussion_r4373250592
##########
superset-frontend/packages/superset-ui-chart-controls/src/operators/renameOperator.ts:
##########
@@ -75,7 +91,9 @@ export const renameOperator:
PostProcessingFactory<PostProcessingRename> = (
: [formData.comparison_type, metricOnly, metricWithOffset].join(
TIME_COMPARISON_SEPARATOR,
),
- metrics.length > 1 ? `${metricOnly}, ${offsetLabel}` : offsetLabel,
+ metricCountWithExtras > 1
Review Comment:
With one displayed metric `count`, a dimension `region`, a Values comparison
against `1 year ago`, and a Sort By SQL metric whose label is `count, 1 year
ago`, the sort-only metric now makes `metricCountWithExtras` 2. The rename maps
`count__1 year ago` to `count, 1 year ago`, which is the hidden metric's own
top-level pivot label for the same dimension values. The backend `rename` then
rejects the duplicate with "Label already exists" and the chart fails to load
instead of ignoring the hidden metric. Could the target label be checked
against the pivoted labels (including the extra metrics) before it is used,
with a test for that collision?
--
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]
