fix: save columns reference from sqllab save datasets flow

superset-frontend/packages/superset-ui-chart-controls/src/constants.ts

@@ -48,7 +48,7 @@ export const DATASET_TIME_COLUMN_OPTION: ColumnMeta = {
 };
 
 export const QUERY_TIME_COLUMN_OPTION: QueryColumn = {
-  name: DTTM_ALIAS,
+  column_name: DTTM_ALIAS,
   type: DatasourceType.Query,
   is_dttm: false,
 };

superset-frontend/packages/superset-ui-chart-controls/src/utils/columnChoices.ts

@@ -16,8 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { ensureIsArray, QueryResponse } from '@superset-ui/core';
-import { Dataset, isColumnMeta, isDataset, isQueryResponse } from '../types';
+import { QueryResponse } from '@superset-ui/core';
+import { Dataset, isColumnMeta, isDataset } from '../types';
 
 /**
  * Convert Datasource columns to column choices
@@ -35,14 +35,5 @@ export default function columnChoices(
         opt1[1].toLowerCase() > opt2[1].toLowerCase() ? 1 : -1,
       );
   }
-
-  if (isQueryResponse(datasource)) {
-    return ensureIsArray(datasource.columns)
-      .map((col): [string, string] => [col.name, col.name])
-      .sort((opt1, opt2) =>
-        opt1[1].toLowerCase() > opt2[1].toLowerCase() ? 1 : -1,
-      );
-  }
-
   return [];
 }

superset-frontend/packages/superset-ui-chart-controls/test/utils/getTemporalColumns.test.ts

@@ -54,7 +54,7 @@ test('get temporal columns from a QueryResponse', () => {
   expect(getTemporalColumns(testQueryResponse)).toEqual({
     temporalColumns: [
       {
-        name: 'Column 2',
+        column_name: 'Column 2',
         type: 'TIMESTAMP',
         is_dttm: true,
       },

superset-frontend/packages/superset-ui-core/src/query/types/Query.ts

@@ -247,8 +247,8 @@ export const CtasEnum = {
 };
 
 export type QueryColumn = {
-  name: string;
-  column_name?: string;
+  name?: string;
+  column_name: string;
   type: string | null;
   is_dttm: boolean;
 };
@@ -380,17 +380,17 @@ export const testQuery: Query = {
   type: DatasourceType.Query,
   columns: [
     {
-      name: 'Column 1',
+      column_name: 'Column 1',
       type: 'STRING',
       is_dttm: false,
     },
     {
-      name: 'Column 3',
+      column_name: 'Column 3',
       type: 'STRING',
       is_dttm: false,
     },
     {
-      name: 'Column 2',
+      column_name: 'Column 2',
       type: 'TIMESTAMP',
       is_dttm: true,
     },
@@ -402,17 +402,17 @@ export const testQueryResults = {
     displayLimitReached: false,
     columns: [
       {
-        name: 'Column 1',
+        column_name: 'Column 1',
         type: 'STRING',
         is_dttm: false,
       },
       {
-        name: 'Column 3',
+        column_name: 'Column 3',
         type: 'STRING',
         is_dttm: false,
       },
       {
-        name: 'Column 2',
+        column_name: 'Column 2',
         type: 'TIMESTAMP',
         is_dttm: true,
       },
@@ -423,17 +423,17 @@ export const testQueryResults = {
     expanded_columns: [],
     selected_columns: [
       {
-        name: 'Column 1',
+        column_name: 'Column 1',
         type: 'STRING',
         is_dttm: false,
       },
       {
-        name: 'Column 3',
+        column_name: 'Column 3',
         type: 'STRING',
         is_dttm: false,
       },
       {
-        name: 'Column 2',
+        column_name: 'Column 2',
         type: 'TIMESTAMP',
         is_dttm: true,
       },

superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx

@@ -122,10 +122,10 @@ describe('ResultSet', () => {
     expect(table).toBeInTheDocument();
 
     const firstColumn = queryAllByText(
-      mockedProps.query.results?.columns[0].name ?? '',
+      mockedProps.query.results?.columns[0].column_name ?? '',
     )[0];
     const secondColumn = queryAllByText(
-      mockedProps.query.results?.columns[1].name ?? '',
+      mockedProps.query.results?.columns[1].column_name ?? '',
     )[0];
     expect(firstColumn).toBeInTheDocument();
     expect(secondColumn).toBeInTheDocument();

superset-frontend/src/SqlLab/components/ResultSet/index.tsx

@@ -205,7 +205,7 @@ const ResultSet = ({
         ...EXPLORE_CHART_DEFAULT,
         datasource: `${results.query_id}__query`,
         ...{
-          all_columns: results.columns.map(column => column.name),
+          all_columns: results.columns.map(column => column.column_name),
         },
       });
       const url = mountExploreUrl(null, {
@@ -491,7 +491,7 @@ const ResultSet = ({
     }
     if (data && data.length > 0) {
       const expandedColumns = results.expanded_columns
-        ? results.expanded_columns.map(col => col.name)
+        ? results.expanded_columns.map(col => col.column_name)
         : [];
       return (
         <>
@@ -500,7 +500,7 @@ const ResultSet = ({
           {sql}
           <FilterableTable
             data={data}
-            orderedColumnKeys={results.columns.map(col => col.name)}
+            orderedColumnKeys={results.columns.map(col => col.column_name)}
             height={rowsHeight}
             filterText={searchText}
             expandedColumns={expandedColumns}

superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx

@@ -62,6 +62,7 @@ export type ExploreQuery = QueryResponse & {
 
 export interface ISimpleColumn {
   column_name?: string | null;
+  name?: string | null;
   type?: string | null;
   is_dttm?: boolean | null;
 }
@@ -199,14 +200,15 @@ export const SaveDatasetModal = ({
       return;
     }
     setLoading(true);
+
     const [, key] = await Promise.all([
       updateDataset(
         datasource?.dbId,
         datasetToOverwrite?.datasetid,
         datasource?.sql,
         datasource?.columns?.map(
-          (d: { name: string; type: string; is_dttm: boolean }) => ({
-            column_name: d.name,
+          (d: { column_name: string; type: string; is_dttm: boolean }) => ({
+            column_name: d.column_name,
             type: d.type,
             is_dttm: d.is_dttm,
           }),
@@ -292,12 +294,10 @@ export const SaveDatasetModal = ({
 
     dispatch(
       createDatasource({
-        schema: datasource.schema,
         sql: datasource.sql,
         dbId: datasource.dbId || datasource?.database?.id,
         templateParams,
         datasourceName: datasetName,
-        columns: selectedColumns,
       }),
     )
       .then((data: { id: number }) =>

superset-frontend/src/SqlLab/fixtures.ts

@@ -236,24 +236,24 @@ export const queries = [
       columns: [
         {
           is_dttm: true,
-          name: 'ds',
+          column_name: 'ds',
           type: 'STRING',
         },
         {
           is_dttm: false,
-          name: 'gender',
+          column_name: 'gender',
           type: 'STRING',
         },
       ],
       selected_columns: [
         {
           is_dttm: true,
-          name: 'ds',
+          column_name: 'ds',
           type: 'STRING',
         },
         {
           is_dttm: false,
-          name: 'gender',
+          column_name: 'gender',
           type: 'STRING',
         },
       ],
@@ -326,7 +326,7 @@ export const queryWithNoQueryLimit = {
     columns: [
       {
         is_dttm: true,
-        name: 'ds',
+        column_name: 'ds',
         type: 'STRING',
       },
       {
@@ -338,12 +338,12 @@ export const queryWithNoQueryLimit = {
     selected_columns: [
       {
         is_dttm: true,
-        name: 'ds',
+        column_name: 'ds',
         type: 'STRING',
       },
       {
         is_dttm: false,
-        name: 'gender',
+        column_name: 'gender',
         type: 'STRING',
       },
     ],
@@ -364,57 +364,57 @@ export const queryWithBadColumns = {
     selected_columns: [
       {
         is_dttm: true,
-        name: 'COUNT(*)',
+        column_name: 'COUNT(*)',
         type: 'STRING',
       },
       {
         is_dttm: false,
-        name: 'this_col_is_ok',
+        column_name: 'this_col_is_ok',
         type: 'STRING',
       },
       {
         is_dttm: false,
-        name: 'a',
+        column_name: 'a',
         type: 'STRING',
       },
       {
         is_dttm: false,
-        name: '1',
+        column_name: '1',
         type: 'STRING',
       },
       {
         is_dttm: false,
-        name: '123',
+        column_name: '123',
         type: 'STRING',
       },
       {
         is_dttm: false,
-        name: 'CASE WHEN 1=1 THEN 1 ELSE 0 END',
+        column_name: 'CASE WHEN 1=1 THEN 1 ELSE 0 END',
         type: 'STRING',
       },
       {
         is_dttm: true,
-        name: '_TIMESTAMP',
+        column_name: '_TIMESTAMP',
         type: 'TIMESTAMP',
       },
       {
         is_dttm: true,
-        name: '__TIME',
+        column_name: '__TIME',
         type: 'TIMESTAMP',
       },
       {
         is_dttm: false,
-        name: 'my_dupe_col__2',
+        column_name: 'my_dupe_col__2',
         type: 'STRING',
       },
       {
         is_dttm: true,
-        name: '__timestamp',
+        column_name: '__timestamp',
         type: 'TIMESTAMP',
       },
       {
         is_dttm: true,
-        name: '__TIMESTAMP',
+        column_name: '__TIMESTAMP',
         type: 'TIMESTAMP',
       },
     ],
@@ -572,31 +572,31 @@ const baseQuery: QueryResponse = {
     columns: [
       {
         is_dttm: true,
-        name: 'ds',
+        column_name: 'ds',
         type: 'STRING',
       },
       {
         is_dttm: false,
-        name: 'gender',
+        column_name: 'gender',
         type: 'STRING',
       },
     ],
     selected_columns: [
       {
         is_dttm: true,
-        name: 'ds',
+        column_name: 'ds',
         type: 'STRING',
       },
       {
         is_dttm: false,
-        name: 'gender',
+        column_name: 'gender',
         type: 'STRING',
       },
     ],
     expanded_columns: [
       {
         is_dttm: true,
-        name: 'ds',
+        column_name: 'ds',
         type: 'STRING',
       },
     ],

superset-frontend/src/components/Datasource/DatasourceEditor.jsx

@@ -688,8 +688,9 @@ class DatasourceEditor extends React.PureComponent {
   }
 
   updateColumns(cols) {
+    // cols: Array<{column_name: string; is_dttm: boolean; type: string;}>
     const { databaseColumns } = this.state;
-    const databaseColumnNames = cols.map(col => col.name);
+    const databaseColumnNames = cols.map(col => col.column_name);
     const currentCols = databaseColumns.reduce(
       (agg, col) => ({
         ...agg,
@@ -706,18 +707,18 @@ class DatasourceEditor extends React.PureComponent {
         .filter(col => !databaseColumnNames.includes(col)),
     };
     cols.forEach(col => {
-      const currentCol = currentCols[col.name];
+      const currentCol = currentCols[col.column_name];
       if (!currentCol) {
         // new column
         finalColumns.push({
           id: shortid.generate(),
-          column_name: col.name,
+          column_name: col.column_name,
           type: col.type,
           groupby: true,
           filterable: true,
           is_dttm: col.is_dttm,
         });
-        results.added.push(col.name);
+        results.added.push(col.column_name);
       } else if (
         currentCol.type !== col.type ||
         (!currentCol.is_dttm && col.is_dttm)
@@ -728,7 +729,7 @@ class DatasourceEditor extends React.PureComponent {
           type: col.type,
           is_dttm: currentCol.is_dttm || col.is_dttm,
         });
-        results.modified.push(col.name);
+        results.modified.push(col.column_name);
       } else {
         // unchanged
         finalColumns.push(currentCol);