From 4cb0260584d599824c7bcb3f7b5a42946b685130 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 10 Sep 2021 11:32:28 +0300 Subject: [PATCH 1/3] feat(plugin-chart-echarts): add support for non-temporal series limit --- .../src/shared-controls/index.tsx | 17 +++++++++++++++++ .../src/query/buildQueryObject.ts | 6 ++++++ .../superset-ui-core/src/query/types/Query.ts | 9 +++++++-- .../src/query/types/QueryFormData.ts | 4 ++++ .../src/BoxPlot/buildQuery.ts | 18 +++++++++++++----- .../src/BoxPlot/controlPanel.ts | 5 +++-- .../test/BoxPlot/buildQuery.test.ts | 17 +++++++++-------- 7 files changed, 59 insertions(+), 17 deletions(-) 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 33f644d9f3..3e9604ef8b 100644 --- a/packages/superset-ui-chart-controls/src/shared-controls/index.tsx +++ b/packages/superset-ui-chart-controls/src/shared-controls/index.tsx @@ -340,6 +340,20 @@ const limit: SharedControlConfig<'SelectControl'> = { ), }; +const series_limit: SharedControlConfig<'SelectControl'> = { + type: 'SelectControl', + freeForm: true, + label: t('Series limit'), + validators: [legacyValidateInteger], + choices: formatSelectOptions(SERIES_LIMITS), + description: t( + 'Limits the number of series that get displayed. A sub query ' + + '(or an extra phase where sub queries are not supported) is applied to limit ' + + 'the number of series that get fetched and displayed. This feature is useful ' + + 'when grouping by high cardinality dimension(s).', + ), +}; + const sort_by: SharedControlConfig<'MetricsControl'> = { type: 'MetricsControl', label: t('Sort By'), @@ -495,6 +509,9 @@ const sharedControls = { adhoc_filters: enableExploreDnd ? dnd_adhoc_filters : adhoc_filters, color_scheme, label_colors, + series_columns: enableExploreDnd ? dndColumnsControl : columnsControl, + series_limit, + series_limit_metric: enableExploreDnd ? dnd_sort_by : sort_by, }; export default sharedControls; diff --git a/packages/superset-ui-core/src/query/buildQueryObject.ts b/packages/superset-ui-core/src/query/buildQueryObject.ts index 801f179ad5..b0f3dcc1ff 100644 --- a/packages/superset-ui-core/src/query/buildQueryObject.ts +++ b/packages/superset-ui-core/src/query/buildQueryObject.ts @@ -32,6 +32,9 @@ export default function buildQueryObject( granularity, url_params = {}, custom_params = {}, + series_columns, + series_limit, + series_limit_metric, ...residualFormData } = formData; const { @@ -76,6 +79,9 @@ export default function buildQueryObject( annotation_layers, row_limit: row_limit == null || Number.isNaN(numericRowLimit) ? undefined : numericRowLimit, row_offset: row_offset == null || Number.isNaN(numericRowOffset) ? undefined : numericRowOffset, + series_columns, + series_limit, + series_limit_metric, timeseries_limit: limit ? Number(limit) : 0, timeseries_limit_metric: timeseries_limit_metric || undefined, order_desc: typeof order_desc === 'undefined' ? true : order_desc, diff --git a/packages/superset-ui-core/src/query/types/Query.ts b/packages/superset-ui-core/src/query/types/Query.ts index ada5627c2e..952f8bc513 100644 --- a/packages/superset-ui-core/src/query/types/Query.ts +++ b/packages/superset-ui-core/src/query/types/Query.ts @@ -21,7 +21,7 @@ import { DatasourceType } from './Datasource'; import { BinaryOperator, SetOperator, UnaryOperator } from './Operator'; import { AppliedTimeExtras, TimeRange, TimeRangeEndpoints } from './Time'; import { AnnotationLayer } from './AnnotationLayer'; -import { QueryFields, QueryFormMetric } from './QueryFormData'; +import { QueryFields, QueryFormColumn, QueryFormMetric } from './QueryFormData'; import { Maybe } from '../../types'; import { PostProcessingRule } from './PostProcessing'; import { JsonObject } from '../../connection'; @@ -121,7 +121,7 @@ export interface QueryObject extends QueryFields, TimeRange, ResidualQueryObject /** The size of bucket by which to group timeseries data (forthcoming) */ time_grain?: string; - /** Maximum number of series */ + /** Maximum number of timeseries */ timeseries_limit?: number; /** The metric used to sort the returned result. */ @@ -136,6 +136,11 @@ export interface QueryObject extends QueryFields, TimeRange, ResidualQueryObject /** Free-form WHERE SQL: multiple clauses are concatenated by AND */ where?: string; + + /** Limit number of series */ + series_columns?: QueryFormColumn[]; + series_limit?: number; + series_limit_metric?: Maybe; } export interface QueryContext { diff --git a/packages/superset-ui-core/src/query/types/QueryFormData.ts b/packages/superset-ui-core/src/query/types/QueryFormData.ts index f7ba4f85dc..b92c466ac8 100644 --- a/packages/superset-ui-core/src/query/types/QueryFormData.ts +++ b/packages/superset-ui-core/src/query/types/QueryFormData.ts @@ -176,6 +176,10 @@ export interface BaseFormData extends TimeRange, FormDataResidual { annotation_layers?: AnnotationLayer[]; url_params?: Record; custom_params?: Record; + /** limit number of series */ + series_columns?: QueryFormColumn[]; + series_limit?: number; + series_limit_metric?: QueryFormColumn; } /** diff --git a/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts b/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts index 30b3cdc053..12bab40a81 100644 --- a/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts +++ b/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts @@ -22,14 +22,14 @@ import { BoxPlotQueryFormData, BoxPlotQueryObjectWhiskerType } from './types'; const PERCENTILE_REGEX = /(\d+)\/(\d+) percentiles/; export default function buildQuery(formData: BoxPlotQueryFormData) { - const { whiskerOptions, columns: distributionColumns = [] } = formData; + const { columns = [], granularity_sqla, groupby = [], whiskerOptions } = formData; return buildQueryContext(formData, baseQueryObject => { let whiskerType: BoxPlotQueryObjectWhiskerType; let percentiles: [number, number] | undefined; - const { columns = [], metrics = [] } = baseQueryObject; + const { metrics = [] } = baseQueryObject; const percentileMatch = PERCENTILE_REGEX.exec(whiskerOptions as string); - if (whiskerOptions === 'Tukey') { + if (whiskerOptions === 'Tukey' || whiskerOptions === undefined) { whiskerType = 'tukey'; } else if (whiskerOptions === 'Min/max (no outliers)') { whiskerType = 'min/max'; @@ -39,17 +39,25 @@ export default function buildQuery(formData: BoxPlotQueryFormData) { } else { throw new Error(`Unsupported whisker type: ${whiskerOptions}`); } + const distributionColumns: string[] = []; + + // For now default to using the temporal column as distribution column. + // In the future this control should be made mandatory. + if (!columns.length && granularity_sqla) { + distributionColumns.push(granularity_sqla); + } return [ { ...baseQueryObject, - is_timeseries: distributionColumns.length === 0, + columns: [...distributionColumns, ...columns, ...groupby], + series_columns: groupby, post_processing: [ { operation: 'boxplot', options: { whisker_type: whiskerType, percentiles, - groupby: columns.filter(x => !distributionColumns.includes(x)), + groupby, metrics: metrics.map(getMetricLabel), }, }, diff --git a/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts b/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts index 4aeb1672e6..eb8dcaca1c 100644 --- a/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts +++ b/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts @@ -37,8 +37,9 @@ export default { ['adhoc_filters'], emitFilterControl, ['groupby'], - ['columns'], - ['limit'], + ['columns'], // this should be migrated to `series_columns` + ['series_limit'], + ['series_limit_metric'], [ { name: 'whiskerOptions', diff --git a/plugins/plugin-chart-echarts/test/BoxPlot/buildQuery.test.ts b/plugins/plugin-chart-echarts/test/BoxPlot/buildQuery.test.ts index 50eb5230a4..96d45fde4a 100644 --- a/plugins/plugin-chart-echarts/test/BoxPlot/buildQuery.test.ts +++ b/plugins/plugin-chart-echarts/test/BoxPlot/buildQuery.test.ts @@ -21,30 +21,31 @@ import { BoxPlotQueryFormData } from '../../src/BoxPlot/types'; describe('BoxPlot buildQuery', () => { const formData: BoxPlotQueryFormData = { + emitFilter: false, + columns: [], datasource: '5__table', granularity_sqla: 'ds', - time_grain_sqla: 'P1Y', - columns: [], - metrics: ['foo'], groupby: ['bar'], + metrics: ['foo'], + time_grain_sqla: 'P1Y', + viz_type: 'my_chart', whiskerOptions: 'Tukey', yAxisFormat: 'SMART_NUMBER', - viz_type: 'my_chart', }; - it('should build timeseries when columns is empty', () => { + it('should build timeseries when series columns is empty', () => { const queryContext = buildQuery(formData); const [query] = queryContext.queries; - expect(query.is_timeseries).toEqual(true); expect(query.metrics).toEqual(['foo']); - expect(query.columns).toEqual(['bar']); + expect(query.columns).toEqual(['ds', 'bar']); + expect(query.series_columns).toEqual(['bar']); }); it('should build non-timeseries query object when columns is defined', () => { const queryContext = buildQuery({ ...formData, columns: ['qwerty'] }); const [query] = queryContext.queries; - expect(query.is_timeseries).toEqual(false); expect(query.metrics).toEqual(['foo']); expect(query.columns).toEqual(['qwerty', 'bar']); + expect(query.series_columns).toEqual(['bar']); }); }); From 193d3701e1e551fe3245e995a8291bedf2e10f81 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Fri, 10 Sep 2021 15:36:10 +0300 Subject: [PATCH 2/3] Update plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts b/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts index eb8dcaca1c..ecb726e911 100644 --- a/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts +++ b/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts @@ -37,7 +37,7 @@ export default { ['adhoc_filters'], emitFilterControl, ['groupby'], - ['columns'], // this should be migrated to `series_columns` + ['columns'], // TODO: this should be migrated to `series_columns` ['series_limit'], ['series_limit_metric'], [ From 3a8397cf36f16cee8aa24b5eb264b1fca04bd090 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 10 Sep 2021 16:06:34 +0300 Subject: [PATCH 3/3] make whisker type unclearable and also handle null case --- plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts | 2 +- plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts b/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts index 12bab40a81..c31ceaf703 100644 --- a/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts +++ b/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts @@ -29,7 +29,7 @@ export default function buildQuery(formData: BoxPlotQueryFormData) { const { metrics = [] } = baseQueryObject; const percentileMatch = PERCENTILE_REGEX.exec(whiskerOptions as string); - if (whiskerOptions === 'Tukey' || whiskerOptions === undefined) { + if (whiskerOptions === 'Tukey' || !whiskerOptions) { whiskerType = 'tukey'; } else if (whiskerOptions === 'Min/max (no outliers)') { whiskerType = 'min/max'; diff --git a/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts b/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts index ecb726e911..0df2841a08 100644 --- a/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts +++ b/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts @@ -44,6 +44,7 @@ export default { { name: 'whiskerOptions', config: { + clearable: false, type: 'SelectControl', freeForm: true, label: t('Whisker/outlier options'),