diff --git a/packages/superset-ui-chart-controls/src/constants.ts b/packages/superset-ui-chart-controls/src/constants.ts index b9934ada50..5661492d27 100644 --- a/packages/superset-ui-chart-controls/src/constants.ts +++ b/packages/superset-ui-chart-controls/src/constants.ts @@ -17,6 +17,7 @@ * under the License. */ import { t } from '@superset-ui/core'; +import { ColumnMeta } from './types'; // eslint-disable-next-line import/prefer-default-export export const TIME_FILTER_LABELS = { @@ -26,3 +27,9 @@ export const TIME_FILTER_LABELS = { druid_time_origin: t('Origin'), granularity: t('Time Granularity'), }; + +export const TIME_COLUMN_OPTION: ColumnMeta = { + verbose_name: t('Time'), + column_name: '__timestamp', + description: t('A reference to the [Time] configuration, taking granularity into account'), +}; diff --git a/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx b/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx index 30bb7d7b18..cd250cbfa5 100644 --- a/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx +++ b/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx @@ -19,12 +19,7 @@ */ import { t, validateNonEmpty } from '@superset-ui/core'; import { ExtraControlProps, SharedControlConfig } from '../types'; - -const timeColumnOption = { - verbose_name: t('Time'), - column_name: '__timestamp', - description: t('A reference to the [Time] configuration, taking granularity into account'), -}; +import { TIME_COLUMN_OPTION } from '../constants'; export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = { type: 'DndColumnSelect', @@ -36,7 +31,7 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = { if (state.datasource) { const options = state.datasource.columns.filter(c => c.groupby); if (includeTime) { - options.unshift(timeColumnOption); + options.unshift(TIME_COLUMN_OPTION); } newState.options = Object.fromEntries(options.map(option => [option.column_name, option])); } diff --git a/packages/superset-ui-chart-controls/src/shared-controls/index.tsx b/packages/superset-ui-chart-controls/src/shared-controls/index.tsx index 5a9744469f..579c1b2e23 100644 --- a/packages/superset-ui-chart-controls/src/shared-controls/index.tsx +++ b/packages/superset-ui-chart-controls/src/shared-controls/index.tsx @@ -47,7 +47,7 @@ import { } from '@superset-ui/core'; import { mainMetric, formatSelectOptions } from '../utils'; -import { TIME_FILTER_LABELS } from '../constants'; +import { TIME_FILTER_LABELS, TIME_COLUMN_OPTION } from '../constants'; import { Metric, SharedControlConfig, @@ -103,12 +103,6 @@ export const D3_TIME_FORMAT_OPTIONS = [ export const D3_TIME_FORMAT_DOCS = t('D3 time format syntax: https://github.com/d3/d3-time-format'); -const timeColumnOption = { - verbose_name: t('Time'), - column_name: '__timestamp', - description: t('A reference to the [Time] configuration, taking granularity into account'), -}; - type Control = { savedMetrics?: Metric[] | null; default?: unknown; @@ -137,7 +131,7 @@ const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = { if (state.datasource) { const options = state.datasource.columns.filter(c => c.groupby); if (includeTime) { - options.unshift(timeColumnOption); + options.unshift(TIME_COLUMN_OPTION); } newState.options = options; } diff --git a/packages/superset-ui-chart-controls/src/types.ts b/packages/superset-ui-chart-controls/src/types.ts index 180a6752ef..9d5b91d5fb 100644 --- a/packages/superset-ui-chart-controls/src/types.ts +++ b/packages/superset-ui-chart-controls/src/types.ts @@ -18,7 +18,7 @@ * under the License. */ import React, { ReactNode, ReactText, ReactElement } from 'react'; -import { QueryFormData, DatasourceType, Metric, JsonValue } from '@superset-ui/core'; +import { QueryFormData, DatasourceType, Metric, JsonValue, Column } from '@superset-ui/core'; import sharedControls from './shared-controls'; import sharedControlComponents from './shared-controls/components'; @@ -38,16 +38,10 @@ export type SharedControlComponents = typeof sharedControlComponents; /** ---------------------------------------------- * Input data/props while rendering * ---------------------------------------------*/ -export interface ColumnMeta extends AnyDict { - column_name: string; - groupby?: string; - verbose_name?: string; - description?: string; - expression?: string; - is_dttm?: boolean; +export type ColumnMeta = Omit & { + id?: number; type?: string; - filterable?: boolean; -} +} & AnyDict; export interface DatasourceMeta { id: number; diff --git a/packages/superset-ui-core/src/query/extractQueryFields.ts b/packages/superset-ui-core/src/query/extractQueryFields.ts index 8386abb826..ec61ce629a 100644 --- a/packages/superset-ui-core/src/query/extractQueryFields.ts +++ b/packages/superset-ui-core/src/query/extractQueryFields.ts @@ -20,7 +20,15 @@ import { t } from '../translation'; import { removeDuplicates } from '../utils'; import { DTTM_ALIAS } from './buildQueryObject'; import getMetricLabel from './getMetricLabel'; -import { QueryFields, QueryFieldAliases, FormDataResidual, QueryMode } from './types/QueryFormData'; +import { + QueryFields, + QueryFormColumn, + QueryFormMetric, + QueryFormOrderBy, + QueryFieldAliases, + FormDataResidual, + QueryMode, +} from './types/QueryFormData'; /** * Extra SQL query related fields from chart form data. @@ -47,13 +55,12 @@ export default function extractQueryFields( order_by_cols: 'orderby', ...aliases, }; - const finalQueryFields: QueryFields = { - columns: [], - metrics: [], - orderby: [], - }; const { query_mode: queryMode, include_time: includeTime, ...restFormData } = formData; + let columns: QueryFormColumn[] = []; + let metrics: QueryFormMetric[] = []; + let orderby: QueryFormOrderBy[] = []; + Object.entries(restFormData).forEach(([key, value]) => { // ignore `null` or `undefined` value if (value == null) { @@ -62,14 +69,6 @@ export default function extractQueryFields( let normalizedKey: string = queryFieldAliases[key] || key; - // ignore groupby and metrics when in raw records mode - if ( - queryMode === QueryMode.raw && - (normalizedKey === 'groupby' || normalizedKey === 'metrics') - ) { - return; - } - // ignore columns when (specifically) in aggregate mode. // For charts that support both aggregate and raw records mode, // we store both `groupby` and `columns` in `formData`, so users can @@ -78,42 +77,50 @@ export default function extractQueryFields( return; } + // for the same reason, ignore groupby and metrics in raw records mode + if ( + queryMode === QueryMode.raw && + (normalizedKey === 'groupby' || normalizedKey === 'metrics') + ) { + return; + } + // groupby has been deprecated in QueryObject: https://github.com/apache/superset/pull/9366 - // We translate all `groupby` to `columns`. if (normalizedKey === 'groupby') { normalizedKey = 'columns'; } if (normalizedKey === 'metrics') { - finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey].concat(value); + metrics = metrics.concat(value); } else if (normalizedKey === 'columns') { // currently the columns field only accept pre-defined columns (string shortcut) - finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey] - .concat(value) - .filter(x => typeof x === 'string' && x); + columns = columns.concat(value); } else if (normalizedKey === 'orderby') { - finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey].concat(value).map(item => { - // value can be in the format of `['["col1", true]', '["col2", false]'], - // where the option strings come directly from `order_by_choices`. - if (typeof item === 'string') { - try { - return JSON.parse(item); - } catch (error) { - throw new Error(t('Found invalid orderby options')); - } - } - return item; - }); + orderby = orderby.concat(value); } }); - if (includeTime && !finalQueryFields.columns.includes(DTTM_ALIAS)) { - finalQueryFields.columns.unshift(DTTM_ALIAS); + if (includeTime && !columns.includes(DTTM_ALIAS)) { + columns.unshift(DTTM_ALIAS); } - // remove duplicate columns and metrics - finalQueryFields.columns = removeDuplicates(finalQueryFields.columns); - finalQueryFields.metrics = removeDuplicates(finalQueryFields.metrics, getMetricLabel); - - return finalQueryFields; + return { + columns: removeDuplicates(columns.filter(x => typeof x === 'string' && x)), + metrics: queryMode === QueryMode.raw ? undefined : removeDuplicates(metrics, getMetricLabel), + orderby: + orderby.length > 0 + ? orderby.map(item => { + // value can be in the format of `['["col1", true]', '["col2", false]'], + // where the option strings come directly from `order_by_choices`. + if (typeof item === 'string') { + try { + return JSON.parse(item); + } catch (error) { + throw new Error(t('Found invalid orderby options')); + } + } + return item; + }) + : undefined, + }; } diff --git a/packages/superset-ui-core/src/query/getMetricLabel.ts b/packages/superset-ui-core/src/query/getMetricLabel.ts index fc5fe948db..23adbd0aa7 100644 --- a/packages/superset-ui-core/src/query/getMetricLabel.ts +++ b/packages/superset-ui-core/src/query/getMetricLabel.ts @@ -8,7 +8,7 @@ export default function getMetricLabel(metric: QueryFormMetric) { return metric.label; } if (metric.expressionType === 'SIMPLE') { - return `${metric.aggregate}(${metric.column.columnName})`; + return `${metric.aggregate}(${metric.column.columnName || metric.column.column_name})`; } return metric.sqlExpression; } diff --git a/packages/superset-ui-core/src/query/types/Column.ts b/packages/superset-ui-core/src/query/types/Column.ts index cce4e14721..15a6e04975 100644 --- a/packages/superset-ui-core/src/query/types/Column.ts +++ b/packages/superset-ui-core/src/query/types/Column.ts @@ -41,7 +41,15 @@ export enum ColumnType { export interface Column { id: number; type: ColumnType; - columnName: string; + column_name: string; + groupby?: boolean; + is_dttm?: boolean; + filterable?: boolean; + verbose_name?: string | null; + description?: string | null; + expression?: string | null; + database_expression?: string | null; + python_date_format?: string | null; } export default {}; diff --git a/packages/superset-ui-core/src/query/types/Metric.ts b/packages/superset-ui-core/src/query/types/Metric.ts index bd610a7a1e..e09f2087e0 100644 --- a/packages/superset-ui-core/src/query/types/Metric.ts +++ b/packages/superset-ui-core/src/query/types/Metric.ts @@ -29,7 +29,10 @@ export interface AdhocMetricBase { export interface AdhocMetricSimple extends AdhocMetricBase { expressionType: 'SIMPLE'; - column: Column; + column: Omit & { + column_name?: string; + columnName?: string; + }; aggregate: Aggregate; } diff --git a/packages/superset-ui-core/src/query/types/QueryFormData.ts b/packages/superset-ui-core/src/query/types/QueryFormData.ts index bc4d70b460..1b5de86a7a 100644 --- a/packages/superset-ui-core/src/query/types/QueryFormData.ts +++ b/packages/superset-ui-core/src/query/types/QueryFormData.ts @@ -61,8 +61,8 @@ export enum QueryMode { */ export interface QueryFields { columns: QueryFormColumn[]; - metrics: QueryFormMetric[]; - orderby: QueryFormOrderBy[]; + metrics?: QueryFormMetric[]; + orderby?: QueryFormOrderBy[]; } /** diff --git a/packages/superset-ui-core/test/fixtures.ts b/packages/superset-ui-core/test/fixtures.ts new file mode 100644 index 0000000000..fd5db5c984 --- /dev/null +++ b/packages/superset-ui-core/test/fixtures.ts @@ -0,0 +1,38 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { AdhocMetric, ColumnType } from '../src'; + +export const NUM_METRIC: AdhocMetric = { + expressionType: 'SIMPLE', + label: 'Sum(num)', + column: { + id: 336, + type: ColumnType.BIGINT, + column_name: 'num', + verbose_name: null, + description: null, + expression: '', + filterable: false, + groupby: false, + is_dttm: false, + database_expression: null, + python_date_format: null, + }, + aggregate: 'SUM', +}; diff --git a/packages/superset-ui-core/test/query/extractQueryFields.test.ts b/packages/superset-ui-core/test/query/extractQueryFields.test.ts index ea02d23abe..53b40c7d41 100644 --- a/packages/superset-ui-core/test/query/extractQueryFields.test.ts +++ b/packages/superset-ui-core/test/query/extractQueryFields.test.ts @@ -20,6 +20,7 @@ import extractQueryFields from '@superset-ui/core/src/query/extractQueryFields'; import { configure } from '../../src/translation'; import { QueryMode } from '../../src'; import { DTTM_ALIAS } from '../../src/query/buildQueryObject'; +import { NUM_METRIC } from '../fixtures'; configure(); @@ -28,7 +29,7 @@ describe('extractQueryFields', () => { expect(extractQueryFields({})).toEqual({ columns: [], metrics: [], - orderby: [], + orderby: undefined, }); }); @@ -55,25 +56,35 @@ describe('extractQueryFields', () => { expect(extractQueryFields({ columns: 'col_1' })).toEqual({ columns: ['col_1'], metrics: [], - orderby: [], + orderby: undefined, }); }); - it('should extract groupby', () => { + it('should extract groupby as columns and set empty metrics', () => { expect(extractQueryFields({ groupby: 'col_1' })).toEqual({ columns: ['col_1'], metrics: [], - orderby: [], + orderby: undefined, }); }); - it('should extract custom columns', () => { + it('should remove duplicate metrics', () => { + expect( + extractQueryFields({ metrics: ['col_1', { ...NUM_METRIC }, { ...NUM_METRIC }] }), + ).toEqual({ + columns: [], + metrics: ['col_1', NUM_METRIC], + orderby: undefined, + }); + }); + + it('should extract custom columns fields', () => { expect( extractQueryFields({ series: 'col_1', metric: 'metric_1' }, { series: 'groupby' }), ).toEqual({ columns: ['col_1'], metrics: ['metric_1'], - orderby: [], + orderby: undefined, }); }); @@ -86,7 +97,7 @@ describe('extractQueryFields', () => { ).toEqual({ columns: ['col_1', 'col_2'], metrics: ['metric_1'], - orderby: [], + orderby: undefined, }); }); @@ -113,13 +124,25 @@ describe('extractQueryFields', () => { query_mode: QueryMode.raw, }), ).toEqual({ - metrics: [], columns: ['a'], - orderby: [], + metrics: undefined, + orderby: undefined, }); }); it('should ignore columns when in aggregate QueryMode', () => { + expect( + extractQueryFields({ + columns: ['a'], + groupby: [], + metric: ['m'], + query_mode: QueryMode.aggregate, + }), + ).toEqual({ + metrics: ['m'], + columns: [], + orderby: undefined, + }); expect( extractQueryFields({ columns: ['a'], @@ -130,7 +153,7 @@ describe('extractQueryFields', () => { ).toEqual({ metrics: ['m'], columns: ['b'], - orderby: [], + orderby: undefined, }); }); @@ -151,6 +174,7 @@ describe('extractQueryFields', () => { ], }); }); + it('should throw error if parse orderby failed', () => { expect(() => { extractQueryFields({ diff --git a/packages/superset-ui-core/test/query/getMetricLabel.test.ts b/packages/superset-ui-core/test/query/getMetricLabel.test.ts index d3f80bd206..589b6bffdc 100644 --- a/packages/superset-ui-core/test/query/getMetricLabel.test.ts +++ b/packages/superset-ui-core/test/query/getMetricLabel.test.ts @@ -29,9 +29,23 @@ describe('getMetricLabel', () => { expressionType: 'SIMPLE', aggregate: 'AVG', column: { + id: 5, + type: ColumnType.BIGINT, columnName: 'sum_girls', + }, + }), + ).toEqual('AVG(sum_girls)'); + }); + + it('should handle column_name in alternative field', () => { + expect( + getMetricLabel({ + expressionType: 'SIMPLE', + aggregate: 'AVG', + column: { id: 5, type: ColumnType.BIGINT, + column_name: 'sum_girls', }, }), ).toEqual('AVG(sum_girls)'); diff --git a/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts b/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts index afb9d1432d..e096b382ff 100644 --- a/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts +++ b/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts @@ -26,7 +26,7 @@ export default function buildQuery(formData: BoxPlotQueryFormData) { return buildQueryContext(formData, baseQueryObject => { let whiskerType: BoxPlotQueryObjectWhiskerType; let percentiles: [number, number] | undefined; - const { columns, metrics } = baseQueryObject; + const { columns, metrics = [] } = baseQueryObject; const percentileMatch = PERCENTILE_REGEX.exec(whiskerOptions as string); if (whiskerOptions === 'Tukey') { diff --git a/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts b/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts index c9ad3bb15f..5ffe2df879 100644 --- a/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts +++ b/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts @@ -20,7 +20,7 @@ import { buildQueryContext, getMetricLabel, QueryFormData } from '@superset-ui/c export default function buildQuery(formData: QueryFormData) { return buildQueryContext(formData, baseQueryObject => { - const metricLabels = baseQueryObject.metrics.map(getMetricLabel); + const metricLabels = (baseQueryObject.metrics || []).map(getMetricLabel); return [ { ...baseQueryObject, diff --git a/plugins/plugin-chart-table/src/buildQuery.ts b/plugins/plugin-chart-table/src/buildQuery.ts index cd8e399e3d..2233997aeb 100644 --- a/plugins/plugin-chart-table/src/buildQuery.ts +++ b/plugins/plugin-chart-table/src/buildQuery.ts @@ -59,7 +59,7 @@ const buildQuery: BuildQuery = (formData: TableChartFormData } return buildQueryContext(formDataCopy, baseQueryObject => { - let { metrics, orderby } = baseQueryObject; + let { metrics = [], orderby = [] } = baseQueryObject; let postProcessing: PostProcessingRule[] = []; if (queryMode === QueryMode.aggregate) {