From 8239476068570062dc02193b616414c75a13bf52 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Tue, 2 Mar 2021 17:19:12 +0200 Subject: [PATCH 1/3] feat(core): implement time grain and column filters --- .../src/query/buildQueryObject.ts | 17 ++- .../src/query/processExtraFormData.ts | 64 ++++---- .../src/query/processFilters.ts | 2 +- .../superset-ui-core/src/query/types/Query.ts | 11 +- .../test/query/buildQueryObject.test.ts | 35 +++++ .../test/query/processExtraFormData.test.ts | 141 +++++------------- 6 files changed, 121 insertions(+), 149 deletions(-) diff --git a/packages/superset-ui-core/src/query/buildQueryObject.ts b/packages/superset-ui-core/src/query/buildQueryObject.ts index 0017af96eb..8ee22b3ab1 100644 --- a/packages/superset-ui-core/src/query/buildQueryObject.ts +++ b/packages/superset-ui-core/src/query/buildQueryObject.ts @@ -1,10 +1,11 @@ /* eslint-disable camelcase */ -import { QueryObject } from './types/Query'; +import { QueryObject, QueryObjectFilterClause } from './types/Query'; import { QueryFieldAliases, QueryFormData } from './types/QueryFormData'; import processFilters from './processFilters'; import extractExtras from './extractExtras'; import extractQueryFields from './extractQueryFields'; -import { appendExtraFormData, overrideExtraFormData } from './processExtraFormData'; +import { overrideExtraFormData } from './processExtraFormData'; +import { AdhocFilter } from './types'; export const DTTM_ALIAS = '__timestamp'; @@ -42,8 +43,17 @@ export default function buildQueryObject( const { metrics, columns, orderby } = extractQueryFields(residualFormData, queryFields); const extras = extractExtras(formData); + // collect all filters for conversion to simple filters/freeform clauses + const filterFormData: { + filters: QueryObjectFilterClause[]; + adhoc_filters: AdhocFilter[]; + } = { + filters: [...(formData.filters || []), ...(append_form_data.filters || [])], + adhoc_filters: [...(formData.adhoc_filters || []), ...(append_form_data.adhoc_filters || [])], + }; const extrasAndfilters = processFilters({ ...formData, + ...filterFormData, ...extras, }); @@ -68,8 +78,7 @@ export default function buildQueryObject( url_params: url_params || undefined, custom_params, }; - // append and override extra form data used by native filters - queryObject = appendExtraFormData(queryObject, append_form_data); + // override extra form data used by native and cross filters queryObject = overrideExtraFormData(queryObject, override_form_data); return { ...queryObject, custom_form_data }; } diff --git a/packages/superset-ui-core/src/query/processExtraFormData.ts b/packages/superset-ui-core/src/query/processExtraFormData.ts index a7201978d6..5eab6ce5fd 100644 --- a/packages/superset-ui-core/src/query/processExtraFormData.ts +++ b/packages/superset-ui-core/src/query/processExtraFormData.ts @@ -16,55 +16,43 @@ * specific language governing permissions and limitations * under the License. */ -import { QueryObject } from './types/Query'; +import { QueryObject, QueryObjectExtras, SqlaFormData } from './types'; -const ALLOWED_OVERRIDES = ['time_grain_sqla', 'time_range', 'since', 'until']; -const ALLOWED_APPENDS = ['adhoc_filters', 'filters', 'groupby']; +const ALLOWED_OVERRIDES: Record = { + granularity: 'granularity', + granularity_sqla: 'granularity', + time_column: 'time_column', + time_grain: 'time_grain', + time_range: 'time_range', +}; +const ALLOWED_EXTRAS_OVERRIDES: (keyof QueryObjectExtras)[] = [ + 'druid_time_origin', + 'relative_start', + 'relative_end', + 'time_grain_sqla', + 'time_range_endpoints', +]; export function overrideExtraFormData( queryObject: QueryObject, overrideFormData: Partial, ): QueryObject { - const overriddenFormData = { ...queryObject }; - ALLOWED_OVERRIDES.forEach(key => { + const overriddenFormData: QueryObject = { ...queryObject }; + const { extras: overriddenExtras = {} } = overriddenFormData; + Object.entries(ALLOWED_OVERRIDES).forEach(([key, target]) => { if (key in overrideFormData) { - overriddenFormData[key] = overrideFormData[key]; + overriddenFormData[target] = overrideFormData[key]; } }); - return overriddenFormData; -} - -export function appendExtraFormData( - queryObject: QueryObject, - appendFormData: Partial, -): QueryObject { - const appendedFormData = { ...queryObject }; - ALLOWED_APPENDS.forEach(key => { - if (key in appendFormData) { - const append = appendFormData[key]; - const currentValue = appendedFormData[key] || []; + const { extras: overrideExtras = {} } = overrideFormData; + ALLOWED_EXTRAS_OVERRIDES.forEach(key => { + if (key in overrideExtras) { // @ts-ignore - currentValue.push(...append); - appendedFormData[key] = currentValue; + overriddenExtras[key] = overrideExtras[key]; } }); - - // Add freeform where - const { extras = {} } = appendedFormData; - const { where = '' } = extras; - extras.where = where; - - const { extras: appendExtras = {} } = appendFormData; - let { where: appendWhere } = appendExtras; - - if (appendWhere) { - appendedFormData.extras = extras; - appendWhere = `(${appendWhere})`; - } - if (where) { - appendWhere = appendWhere ? `(${where}) AND ${appendWhere}` : where; + if (Object.keys(overriddenExtras).length > 0) { + overriddenFormData.extras = overriddenExtras; } - extras.where = appendWhere; - - return appendedFormData; + return overriddenFormData; } diff --git a/packages/superset-ui-core/src/query/processFilters.ts b/packages/superset-ui-core/src/query/processFilters.ts index 7db0716694..4a3efb7499 100644 --- a/packages/superset-ui-core/src/query/processFilters.ts +++ b/packages/superset-ui-core/src/query/processFilters.ts @@ -5,7 +5,7 @@ import { isSimpleAdhocFilter } from './types/Filter'; import convertFilter from './convertFilter'; /** Logic formerly in viz.py's process_query_filters */ -export default function processFilters(formData: QueryFormData): Partial { +export default function processFilters(formData: Partial): Partial { // Split adhoc_filters into four fields according to // (1) clause (WHERE or HAVING) // (2) expressionType diff --git a/packages/superset-ui-core/src/query/types/Query.ts b/packages/superset-ui-core/src/query/types/Query.ts index 6e1b4543fa..2ac0bfdd18 100644 --- a/packages/superset-ui-core/src/query/types/Query.ts +++ b/packages/superset-ui-core/src/query/types/Query.ts @@ -85,12 +85,15 @@ export interface QueryObject extends QueryFields, TimeRange, ResidualQueryObject /** SIMPLE where filters */ filters?: QueryObjectFilterClause[]; - /** Time column. */ + /** Time column for SQL, time-grain for Druid (deprecated) */ granularity?: string; /** If set, will group by timestamp */ is_timeseries?: boolean; + /** Should the rowcount of the query be fetched */ + is_rowcount?: boolean; + /** Free-form HAVING SQL, multiple clauses are concatenated by AND */ having?: string; @@ -105,6 +108,12 @@ export interface QueryObject extends QueryFields, TimeRange, ResidualQueryObject /** Number of rows to skip */ row_offset?: number; + /** The column to which direct temporal filters (forthcoming) */ + time_column?: string; + + /** The size of bucket by which to group timeseries data (forthcoming) */ + time_grain?: string; + /** Maximum number of series */ timeseries_limit?: number; diff --git a/packages/superset-ui-core/test/query/buildQueryObject.test.ts b/packages/superset-ui-core/test/query/buildQueryObject.test.ts index 411391a378..9321ea927e 100644 --- a/packages/superset-ui-core/test/query/buildQueryObject.test.ts +++ b/packages/superset-ui-core/test/query/buildQueryObject.test.ts @@ -58,6 +58,41 @@ describe('buildQueryObject', () => { expect(query.metrics).toEqual(['sum__num', 'avg__num']); }); + it('should merge original and append filters', () => { + query = buildQueryObject({ + datasource: '5__table', + granularity_sqla: 'ds', + viz_type: 'table', + filters: [{ col: 'abc', op: '==', val: 'qwerty' }], + adhoc_filters: [ + { + expressionType: 'SIMPLE', + clause: 'WHERE', + subject: 'foo', + operator: '!=', + comparator: 'bar', + }, + ], + where: 'a = b', + extra_form_data: { + append_form_data: { + adhoc_filters: [ + { + expressionType: 'SQL', + clause: 'WHERE', + sqlExpression: '(1 = 1)', + }, + ], + }, + }, + }); + expect(query.filters).toEqual([ + { col: 'abc', op: '==', val: 'qwerty' }, + { col: 'foo', op: '!=', val: 'bar' }, + ]); + expect(query.extras?.where).toEqual('(a = b) AND ((1 = 1))'); + }); + it('should group custom metric control', () => { query = buildQueryObject( { diff --git a/packages/superset-ui-core/test/query/processExtraFormData.test.ts b/packages/superset-ui-core/test/query/processExtraFormData.test.ts index cd626fa04e..311e04dca0 100644 --- a/packages/superset-ui-core/test/query/processExtraFormData.test.ts +++ b/packages/superset-ui-core/test/query/processExtraFormData.test.ts @@ -16,192 +16,123 @@ * specific language governing permissions and limitations * under the License. */ -import { - appendExtraFormData, - overrideExtraFormData, -} from '@superset-ui/core/src/query/processExtraFormData'; +import { overrideExtraFormData } from '@superset-ui/core/src/query/processExtraFormData'; -describe('appendExtraFormData', () => { - it('should add allowed values to non-existent value', () => { - expect( - appendExtraFormData( - { - datasource: 'table_1', - granularity: 'something', - viz_type: 'custom', - }, - { - filters: [{ col: 'my_col', op: '==', val: 'my value' }], - }, - ), - ).toEqual({ - datasource: 'table_1', - filters: [{ col: 'my_col', op: '==', val: 'my value' }], - granularity: 'something', - viz_type: 'custom', - }); - }); - - it('should add allowed values to preexisting value(s)', () => { +describe('overrideExtraFormData', () => { + it('should assign allowed non-existent value', () => { expect( - appendExtraFormData( + overrideExtraFormData( { granularity: 'something', viz_type: 'custom', datasource: 'table_1', - filters: [{ col: 'my_col', op: '==', val: 'my value' }], }, { - filters: [{ col: 'my_other_col', op: '!=', val: 'my other value' }], + time_range: '100 years ago', }, ), ).toEqual({ granularity: 'something', viz_type: 'custom', datasource: 'table_1', - filters: [ - { col: 'my_col', op: '==', val: 'my value' }, - { col: 'my_other_col', op: '!=', val: 'my other value' }, - ], + time_range: '100 years ago', }); }); - it('should add new freeform where', () => { + it('should override allowed preexisting value', () => { expect( - appendExtraFormData( + overrideExtraFormData( { - datasource: 'table_1', granularity: 'something', viz_type: 'custom', - }, - { - extras: { - where: '1 = 0', - }, - }, - ), - ).toEqual({ - datasource: 'table_1', - granularity: 'something', - viz_type: 'custom', - extras: { - where: '(1 = 0)', - }, - }); - }); - - it('should add new freeform where to existing where clause', () => { - expect( - appendExtraFormData( - { datasource: 'table_1', - granularity: 'something', - viz_type: 'custom', - extras: { - where: 'abc = 1', - }, + time_range: '100 years ago', }, { - extras: { - where: '1 = 0', - }, + time_range: '50 years ago', }, ), ).toEqual({ - datasource: 'table_1', granularity: 'something', viz_type: 'custom', - extras: { - where: '(abc = 1) AND (1 = 0)', - }, - }); - }); - it('should not change existing where if append where is missing', () => { - expect( - appendExtraFormData( - { - datasource: 'table_1', - granularity: 'something', - viz_type: 'custom', - extras: { - where: 'abc = 1', - }, - }, - { - extras: {}, - }, - ), - ).toEqual({ datasource: 'table_1', - granularity: 'something', - viz_type: 'custom', - extras: { - where: 'abc = 1', - }, + time_range: '50 years ago', }); }); -}); -describe('overrideExtraFormData', () => { - it('should assign allowed non-existent value', () => { + it('should not override non-allowed value', () => { expect( overrideExtraFormData( { granularity: 'something', viz_type: 'custom', datasource: 'table_1', + time_range: '100 years ago', }, { - time_grain_sqla: 'PT1H', + viz_type: 'other custom viz', }, ), ).toEqual({ granularity: 'something', viz_type: 'custom', datasource: 'table_1', - time_grain_sqla: 'PT1H', + time_range: '100 years ago', }); }); - it('should override allowed preexisting value', () => { + it('should override pre-existing extra value', () => { expect( overrideExtraFormData( { granularity: 'something', viz_type: 'custom', datasource: 'table_1', - time_grain_sqla: 'PT1H', + time_range: '100 years ago', + extras: { + time_grain_sqla: 'PT1H', + }, }, { - time_grain_sqla: 'PT2H', + extras: { + time_grain_sqla: 'PT2H', + }, }, ), ).toEqual({ granularity: 'something', viz_type: 'custom', datasource: 'table_1', - time_grain_sqla: 'PT2H', + time_range: '100 years ago', + extras: { + time_grain_sqla: 'PT2H', + }, }); }); - it('should not override non-allowed value', () => { + it('should add extra override value', () => { expect( overrideExtraFormData( { granularity: 'something', viz_type: 'custom', datasource: 'table_1', - time_grain_sqla: 'PT1H', + time_range: '100 years ago', }, { - viz_type: 'other custom viz', + extras: { + time_grain_sqla: 'PT1H', + }, }, ), ).toEqual({ granularity: 'something', viz_type: 'custom', datasource: 'table_1', - time_grain_sqla: 'PT1H', + time_range: '100 years ago', + extras: { + time_grain_sqla: 'PT1H', + }, }); }); }); From 555dd3b0c9b3c920cf872902e39dd2c113c71f06 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 5 Mar 2021 23:52:25 +0200 Subject: [PATCH 2/3] add apply_fetch_values_predicate to QueryObject --- .../superset-ui-core/src/query/buildQueryObject.ts | 9 ++++++--- packages/superset-ui-core/src/query/extractExtras.ts | 10 +++++++--- packages/superset-ui-core/src/query/types/Query.ts | 3 +++ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/superset-ui-core/src/query/buildQueryObject.ts b/packages/superset-ui-core/src/query/buildQueryObject.ts index 8ee22b3ab1..301b18f85e 100644 --- a/packages/superset-ui-core/src/query/buildQueryObject.ts +++ b/packages/superset-ui-core/src/query/buildQueryObject.ts @@ -44,17 +44,19 @@ export default function buildQueryObject( const extras = extractExtras(formData); // collect all filters for conversion to simple filters/freeform clauses + const { filters: extraFilters = [] } = extras; + const { adhoc_filters: appendAdhocFilters = [], filters: appendFilters = [] } = append_form_data; const filterFormData: { filters: QueryObjectFilterClause[]; adhoc_filters: AdhocFilter[]; } = { - filters: [...(formData.filters || []), ...(append_form_data.filters || [])], - adhoc_filters: [...(formData.adhoc_filters || []), ...(append_form_data.adhoc_filters || [])], + filters: [...extraFilters, ...appendFilters], + adhoc_filters: [...(formData.adhoc_filters || []), ...appendAdhocFilters], }; const extrasAndfilters = processFilters({ ...formData, - ...filterFormData, ...extras, + ...filterFormData, }); let queryObject: QueryObject = { @@ -80,5 +82,6 @@ export default function buildQueryObject( }; // override extra form data used by native and cross filters queryObject = overrideExtraFormData(queryObject, override_form_data); + return { ...queryObject, custom_form_data }; } diff --git a/packages/superset-ui-core/src/query/extractExtras.ts b/packages/superset-ui-core/src/query/extractExtras.ts index f4eaa6a378..99f0c1ea56 100644 --- a/packages/superset-ui-core/src/query/extractExtras.ts +++ b/packages/superset-ui-core/src/query/extractExtras.ts @@ -1,7 +1,11 @@ /* eslint-disable camelcase */ -import { isDruidFormData, QueryFormData } from './types/QueryFormData'; -import { QueryObject } from './types/Query'; -import { AppliedTimeExtras, TimeColumnConfigKey } from './types/Time'; +import { + AppliedTimeExtras, + isDruidFormData, + QueryFormData, + QueryObject, + TimeColumnConfigKey, +} from './types'; export default function extractExtras(formData: QueryFormData): Partial { const applied_time_extras: AppliedTimeExtras = {}; diff --git a/packages/superset-ui-core/src/query/types/Query.ts b/packages/superset-ui-core/src/query/types/Query.ts index 2ac0bfdd18..0556a7bd3e 100644 --- a/packages/superset-ui-core/src/query/types/Query.ts +++ b/packages/superset-ui-core/src/query/types/Query.ts @@ -76,6 +76,9 @@ export interface QueryObject extends QueryFields, TimeRange, ResidualQueryObject /** Time filters that have been applied to the query object */ applied_time_extras?: AppliedTimeExtras; + /** add fetch value predicate to query if defined in datasource */ + apply_fetch_values_predicate?: boolean; + /** * Extra form data. Current stores information about time granularity, may be * cleaned up in the future. From bd026526dafaeb3b49a09ccb5a0ad32dd47da17f Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Mon, 8 Mar 2021 17:38:02 +0200 Subject: [PATCH 3/3] clean up extract types and fix tests --- .../src/query/buildQueryObject.ts | 4 +- .../src/query/extractExtras.ts | 44 ++++++++---- .../test/query/buildQueryObject.test.ts | 2 +- .../test/query/extractExtras.test.ts | 70 +++++++++++++++---- 4 files changed, 89 insertions(+), 31 deletions(-) diff --git a/packages/superset-ui-core/src/query/buildQueryObject.ts b/packages/superset-ui-core/src/query/buildQueryObject.ts index 301b18f85e..671e9d51b8 100644 --- a/packages/superset-ui-core/src/query/buildQueryObject.ts +++ b/packages/superset-ui-core/src/query/buildQueryObject.ts @@ -42,9 +42,9 @@ export default function buildQueryObject( const numericRowOffset = Number(row_offset); const { metrics, columns, orderby } = extractQueryFields(residualFormData, queryFields); - const extras = extractExtras(formData); // collect all filters for conversion to simple filters/freeform clauses - const { filters: extraFilters = [] } = extras; + const extras = extractExtras(formData); + const { filters: extraFilters } = extras; const { adhoc_filters: appendAdhocFilters = [], filters: appendFilters = [] } = append_form_data; const filterFormData: { filters: QueryObjectFilterClause[]; diff --git a/packages/superset-ui-core/src/query/extractExtras.ts b/packages/superset-ui-core/src/query/extractExtras.ts index 99f0c1ea56..0a2c105724 100644 --- a/packages/superset-ui-core/src/query/extractExtras.ts +++ b/packages/superset-ui-core/src/query/extractExtras.ts @@ -3,20 +3,36 @@ import { AppliedTimeExtras, isDruidFormData, QueryFormData, - QueryObject, + QueryObjectExtras, + QueryObjectFilterClause, TimeColumnConfigKey, } from './types'; -export default function extractExtras(formData: QueryFormData): Partial { +type ExtraFilterQueryField = { + time_range?: string; + granularity_sqla?: string; + time_grain_sqla?: string; + druid_time_origin?: string; + granularity?: string; +}; + +type ExtractedExtra = ExtraFilterQueryField & { + filters: QueryObjectFilterClause[]; + extras: QueryObjectExtras; + applied_time_extras: AppliedTimeExtras; +}; + +export default function extractExtras(formData: QueryFormData): ExtractedExtra { const applied_time_extras: AppliedTimeExtras = {}; - const { extras = {}, filters = [] } = formData; - const partialQueryObject: Partial = { + const filters: QueryObjectFilterClause[] = []; + const extras: QueryObjectExtras = {}; + const extract: ExtractedExtra = { filters, extras, applied_time_extras, }; - const reservedColumnsToQueryField: Record = { + const reservedColumnsToQueryField: Record = { __time_range: 'time_range', __time_col: 'granularity_sqla', __time_grain: 'time_grain_sqla', @@ -28,7 +44,7 @@ export default function extractExtras(formData: QueryFormData): Partial { datasource: '5__table', granularity_sqla: 'ds', viz_type: 'table', - filters: [{ col: 'abc', op: '==', val: 'qwerty' }], + extra_filters: [{ col: 'abc', op: '==', val: 'qwerty' }], adhoc_filters: [ { expressionType: 'SIMPLE', diff --git a/packages/superset-ui-core/test/query/extractExtras.test.ts b/packages/superset-ui-core/test/query/extractExtras.test.ts index e483666c82..4b5f5340b2 100644 --- a/packages/superset-ui-core/test/query/extractExtras.test.ts +++ b/packages/superset-ui-core/test/query/extractExtras.test.ts @@ -24,13 +24,6 @@ describe('extractExtras', () => { granularity_sqla: 'ds', time_grain_sqla: 'PT1M', viz_type: 'my_viz', - filters: [ - { - col: 'gender', - op: '==', - val: 'girl', - }, - ], }; it('should populate time range endpoints and override formData with double underscored date options', () => { @@ -66,13 +59,7 @@ describe('extractExtras', () => { time_grain_sqla: 'PT5M', time_range_endpoints: ['inclusive', 'exclusive'], }, - filters: [ - { - col: 'gender', - op: '==', - val: 'girl', - }, - ], + filters: [], granularity: 'ds2', time_range: '2009-07-17T00:00:00 : 2020-07-17T00:00:00', }); @@ -83,6 +70,11 @@ describe('extractExtras', () => { extractExtras({ ...baseQueryFormData, extra_filters: [ + { + col: 'gender', + op: '==', + val: 'girl', + }, { col: 'name', op: 'IN', @@ -110,4 +102,54 @@ describe('extractExtras', () => { granularity: 'ds', }); }); + + it('should create regular filters from reserved and non-reserved columns', () => { + expect( + extractExtras({ + ...baseQueryFormData, + time_range_endpoints: ['inclusive', 'exclusive'], + extra_filters: [ + { + col: 'gender', + op: '==', + val: 'girl', + }, + { + col: '__time_col', + op: '==', + val: 'ds2', + }, + { + col: '__time_grain', + op: '==', + val: 'PT5M', + }, + { + col: '__time_range', + op: '==', + val: '2009-07-17T00:00:00 : 2020-07-17T00:00:00', + }, + ], + }), + ).toEqual({ + applied_time_extras: { + __time_col: 'ds2', + __time_grain: 'PT5M', + __time_range: '2009-07-17T00:00:00 : 2020-07-17T00:00:00', + }, + extras: { + time_grain_sqla: 'PT5M', + time_range_endpoints: ['inclusive', 'exclusive'], + }, + filters: [ + { + col: 'gender', + op: '==', + val: 'girl', + }, + ], + granularity: 'ds2', + time_range: '2009-07-17T00:00:00 : 2020-07-17T00:00:00', + }); + }); });