EnxDev commented on code in PR #37396:
URL: https://github.com/apache/superset/pull/37396#discussion_r4046924216
##########
superset/utils/core.py:
##########
@@ -1916,6 +1916,42 @@ def extract_dataframe_dtypes(
return generic_types
+def extract_display_labels(
+ label_map: dict[str, list[str]],
+ colnames: list[str],
+ datasource: Explorable | None = None,
+) -> list[str]:
+ """Extract display labels for a list of column names based on a label map
+ and an optional datasource.
+ """
+ if not colnames:
+ return []
+
+ # Build column -> label mapping (skip self-references)
+ columns_to_label = {}
+ if label_map:
+ for label, cols in label_map.items():
+ for col in cols:
+ if label != col and col not in columns_to_label:
+ columns_to_label[col] = label
Review Comment:
Could we avoid applying one output column’s alias to another column here?
`QueryContextProcessor` builds `label_map` from output names to their source
expressions. For a query selecting both `revenue` and an ad hoc column `{label:
"Revenue copy", sqlExpression: "revenue"}`, it produces `{"revenue":
["revenue"], "Revenue copy": ["revenue"]}`. I ran that through this helper and
got `["Revenue copy", "Revenue copy"]`, even with `revenue.verbose_name =
"Revenue"`. The raw column has picked up the other column’s alias. Can we
resolve labels by the output column’s identity and add a regression case with
both the raw column and its aliased copy?
##########
superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx:
##########
@@ -17,60 +17,38 @@
* under the License.
*/
import { useMemo, useCallback, useRef, useState } from 'react';
-import {
- getTimeFormatter,
- TimeFormats,
- getMetricLabel,
- QueryFormMetric,
-} from '@superset-ui/core';
-import { t } from '@apache-superset/core/translation';
+import { getTimeFormatter, TimeFormats } from '@superset-ui/core';
import { Constants } from '@superset-ui/core/components';
import { GenericDataType } from '@apache-superset/core/common';
import type { IRowNode } from 'ag-grid-community';
const timeFormatter = getTimeFormatter(TimeFormats.DATABASE_DATETIME);
-const CONTRIBUTION_SUFFIX = '__contribution';
+/**
+ * Builds Grid column definitions from query result metadata.
+ * Assumes {@link colnames}, {@link coltypes} and {@link collabels}
+ * have the same length and align. Only columns present in the first
+ * data row are included.
+ */
export function useGridColumns(
colnames: string[] | undefined,
coltypes: GenericDataType[] | undefined,
+ collabels: string[] | undefined,
data: Record<string, any>[] | undefined,
- columnDisplayNames?: Record<string, string>,
) {
return useMemo(
() =>
colnames && data?.length
? colnames
- .filter((column: string) => Object.keys(data[0]).includes(column))
- .map((key, index) => {
- const colType = coltypes?.[index];
-
- const rawHeader = columnDisplayNames?.[key] ?? key;
- let cleaned = rawHeader;
- let suffix = '';
-
- if (rawHeader.endsWith(CONTRIBUTION_SUFFIX)) {
- cleaned = rawHeader.slice(
- 0,
- rawHeader.length - CONTRIBUTION_SUFFIX.length,
- );
- suffix = ` (${t('contribution')})`;
- }
-
- try {
- const parsed = JSON.parse(cleaned);
- if (parsed && typeof parsed === 'object') {
- cleaned = getMetricLabel(parsed as QueryFormMetric);
- }
- } catch {
- /* not a JSON-encoded metric – keep original display name */
- }
-
- const cleanHeader = `${cleaned}${suffix}`;
+ .map((column, originalIndex) => [column, originalIndex] as const)
+ .filter(([column]) => Object.keys(data[0]).includes(column))
+ .map(([key, originalIndex]) => {
+ const colType = coltypes?.[originalIndex];
+ const headerLabel = collabels?.[originalIndex];
return {
label: key,
- headerName: cleanHeader,
+ headerName: headerLabel,
Review Comment:
Could we preserve the contribution-header formatting when moving label
resolution to the backend? The pie chart’s `buildQuery` still generates names
such as `sum__num__contribution`, and `extract_display_labels` returns that
name unchanged. With the cleanup removed here, View as table displays
`sum__num__contribution` where it previously displayed `sum__num
(contribution)`. The deleted test in `DataTablesPane.test.tsx` covered this
behavior. Please keep that coverage and either retain the localized suffix
formatting here or provide the equivalent display label from the backend.
--
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]
