feat(core): add extra form data fields for native filters (#992)

* feat(core): implement time grain and column filters

* add apply_fetch_values_predicate to QueryObject

* clean up extract types and fix tests

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';
 
@@ -41,10 +42,21 @@ export default function buildQueryObject<T extends QueryFormData>(
   const numericRowOffset = Number(row_offset);
   const { metrics, columns, orderby } = extractQueryFields(residualFormData, queryFields);
 
+  // collect all filters for conversion to simple filters/freeform clauses
   const extras = extractExtras(formData);
+  const { filters: extraFilters } = extras;
+  const { adhoc_filters: appendAdhocFilters = [], filters: appendFilters = [] } = append_form_data;
+  const filterFormData: {
+    filters: QueryObjectFilterClause[];
+    adhoc_filters: AdhocFilter[];
+  } = {
+    filters: [...extraFilters, ...appendFilters],
+    adhoc_filters: [...(formData.adhoc_filters || []), ...appendAdhocFilters],
+  };
   const extrasAndfilters = processFilters({
     ...formData,
     ...extras,
+    ...filterFormData,
   });
 
   let queryObject: QueryObject = {
@@ -68,8 +80,8 @@ export default function buildQueryObject<T extends QueryFormData>(
     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 };
 }

packages/superset-ui-core/src/query/extractExtras.ts

@@ -1,18 +1,38 @@
 /* eslint-disable camelcase */
-import { isDruidFormData, QueryFormData } from './types/QueryFormData';
-import { QueryObject } from './types/Query';
-import { AppliedTimeExtras, TimeColumnConfigKey } from './types/Time';
+import {
+  AppliedTimeExtras,
+  isDruidFormData,
+  QueryFormData,
+  QueryObjectExtras,
+  QueryObjectFilterClause,
+  TimeColumnConfigKey,
+} from './types';
 
-export default function extractExtras(formData: QueryFormData): Partial<QueryObject> {
+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<QueryObject> = {
+  const filters: QueryObjectFilterClause[] = [];
+  const extras: QueryObjectExtras = {};
+  const extract: ExtractedExtra = {
     filters,
     extras,
     applied_time_extras,
   };
 
-  const reservedColumnsToQueryField: Record<TimeColumnConfigKey, keyof QueryObject> = {
+  const reservedColumnsToQueryField: Record<TimeColumnConfigKey, keyof ExtraFilterQueryField> = {
     __time_range: 'time_range',
     __time_col: 'granularity_sqla',
     __time_grain: 'time_grain_sqla',
@@ -24,28 +44,28 @@ export default function extractExtras(formData: QueryFormData): Partial<QueryObj
     if (filter.col in reservedColumnsToQueryField) {
       const key = filter.col as TimeColumnConfigKey;
       const queryField = reservedColumnsToQueryField[key];
-      partialQueryObject[queryField] = filter.val;
+      extract[queryField] = filter.val as string;
       applied_time_extras[key] = filter.val as string;
     } else {
       filters.push(filter);
     }
   });
 
   // map to undeprecated names and remove deprecated fields
-  if (isDruidFormData(formData) && !partialQueryObject.druid_time_origin) {
+  if (isDruidFormData(formData) && !extract.druid_time_origin) {
     extras.druid_time_origin = formData.druid_time_origin;
-    delete partialQueryObject.druid_time_origin;
+    delete extract.druid_time_origin;
   } else {
     // SQL
-    extras.time_grain_sqla = partialQueryObject.time_grain_sqla || formData.time_grain_sqla;
-    partialQueryObject.granularity =
-      partialQueryObject.granularity_sqla || formData.granularity || formData.granularity_sqla;
-    delete partialQueryObject.granularity_sqla;
-    delete partialQueryObject.time_grain_sqla;
+    extras.time_grain_sqla = extract.time_grain_sqla || formData.time_grain_sqla;
+    extract.granularity =
+      extract.granularity_sqla || formData.granularity || formData.granularity_sqla;
+    delete extract.granularity_sqla;
+    delete extract.time_grain_sqla;
   }
 
   // map time range endpoints:
   if (formData.time_range_endpoints) extras.time_range_endpoints = formData.time_range_endpoints;
 
-  return partialQueryObject;
+  return extract;
 }

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<keyof SqlaFormData, keyof QueryObject> = {
+  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>,
 ): 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>,
-): 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;
 }

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<QueryFormData> {
+export default function processFilters(formData: Partial<QueryFormData>): Partial<QueryFormData> {
   // Split adhoc_filters into four fields according to
   // (1) clause (WHERE or HAVING)
   // (2) expressionType

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.
@@ -85,12 +88,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 +111,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;
 

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',
+      extra_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(
       {

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',
+    });
+  });
 });