Copilot commented on code in PR #39345:
URL: https://github.com/apache/superset/pull/39345#discussion_r3190489076


##########
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:
##########
@@ -796,45 +796,65 @@ const config: ControlPanelConfig = {
                     },
                   );
                 }
-                const { colnames, coltypes } =
+                const { colnames: queryColnames, coltypes: queryColtypes } =
                   chart?.queriesResponse?.[0] ?? {};
-                const allColumns =
-                  Array.isArray(colnames) && Array.isArray(coltypes)
-                    ? [
-                        {
-                          value: ObjectFormattingEnum.ENTIRE_ROW,
-                          label: t('entire row'),
-                          dataType: GenericDataType.String,
-                        },
-                        ...colnames.map((colname: string, index: number) => ({
+                const hasQueryColumns =
+                  Array.isArray(queryColnames) &&
+                  Array.isArray(queryColtypes) &&
+                  queryColnames.length > 0;
+
+                // Fall back to datasource columns when query results are empty
+                const datasourceColumns = ensureIsArray(
+                  (explore?.datasource as Dataset)?.columns,
+                );
+                const colnames = hasQueryColumns
+                  ? queryColnames
+                  : datasourceColumns.map(
+                      (col: ColumnMeta) => col.column_name,
+                    );
+                const coltypes = hasQueryColumns
+                  ? queryColtypes
+                  : datasourceColumns.map(
+                      (col: ColumnMeta) =>
+                        col.type_generic ?? GenericDataType.String,
+                    );
+
+                const hasColumns = colnames.length > 0;
+                const allColumns = hasColumns
+                  ? [
+                      {
+                        value: ObjectFormattingEnum.ENTIRE_ROW,
+                        label: t('entire row'),
+                        dataType: GenericDataType.String,
+                      },
+                      ...colnames.map((colname: string, index: number) => ({
+                        value: colname,
+                        label: Array.isArray(verboseMap)
+                          ? colname
+                          : (verboseMap[colname] ?? colname),
+                        dataType: coltypes[index],
+                      })),
+                    ]
+                  : [];

Review Comment:
   This change makes `ObjectFormattingEnum.ENTIRE_ROW` disappear when there are 
zero columns (e.g., `queriesResponse` has empty `colnames/coltypes` and the 
datasource has no columns). Previously, when `colnames/coltypes` were empty 
arrays, `ENTIRE_ROW` would still be available (because the code only gated on 
`Array.isArray`). Consider always including the `ENTIRE_ROW` option regardless 
of column presence, and only conditionally appending mapped columns; adjust the 
new test(s) accordingly if that behavior is still desired.
   



-- 
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]

Reply via email to