feat(core): add support for adhoc columns (#1342)

* feat(core): add support for adhoc columns * simplify import * revert simplification due to codecov * fix filter column type * fix pivot v2 groupbys * remove redundant import * Add new type guards * move ColumnMeta type guards to chart-controls * add type guard tests * Fix typing and import errors * Fix saved expression type guard * Fix typing * Remove redundant import * Make Echarts and PivotTable handle AdhocColumns properly * Fix lint Co-authored-by: Kamil Gabryjelski <[email protected]>
apache-superset · Nov 10, 2021 · e9780c9 · e9780c9 · vercel · Nov 10, 2021
1 parent 0b948ce
commit e9780c9
Show file tree

Hide file tree

Showing 36 changed files with 315 additions and 80 deletions.
diff --git a/packages/superset-ui-chart-controls/src/operators/boxplotOperator.ts b/packages/superset-ui-chart-controls/src/operators/boxplotOperator.ts
@@ -17,9 +17,10 @@
  * under the License.
  */
 import {
-  PostProcessingBoxplot,
-  getMetricLabel,
   ensureIsArray,
+  getColumnLabel,
+  getMetricLabel,
+  PostProcessingBoxplot,
 } from '@superset-ui/core';
 import { PostProcessingFactory } from './types';
 
@@ -56,7 +57,7 @@ export const boxplotOperator: PostProcessingFactory<
       options: {
         whisker_type: whiskerType,
         percentiles,
-        groupby: ensureIsArray(groupby),
+        groupby: ensureIsArray(groupby).map(getColumnLabel),
         metrics: ensureIsArray(queryObject.metrics).map(getMetricLabel),
       },
     };

diff --git a/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts b/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
@@ -18,6 +18,7 @@
  */
 import {
   ensureIsArray,
+  getColumnLabel,
   getMetricLabel,
   PostProcessingPivot,
 } from '@superset-ui/core';
@@ -38,7 +39,7 @@ export const pivotOperator: PostProcessingFactory<
       operation: 'pivot',
       options: {
         index: [TIME_COLUMN],
-        columns: queryObject.columns || [],
+        columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
         // Create 'dummy' mean aggregates to assign cell values in pivot table
         // use the 'mean' aggregates to avoid drop NaN. PR: https://github.com/apache-superset/superset-ui/pull/1231
         aggregates: Object.fromEntries(

diff --git a/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts b/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts
@@ -21,6 +21,8 @@ import {
   ComparisionType,
   PostProcessingPivot,
   NumpyFunction,
+  ensureIsArray,
+  getColumnLabel,
 } from '@superset-ui/core';
 import {
   getMetricOffsetsMap,
@@ -56,7 +58,7 @@ export const timeComparePivotOperator: PostProcessingFactory<
       operation: 'pivot',
       options: {
         index: ['__timestamp'],
-        columns: queryObject.columns || [],
+        columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
         aggregates:
           comparisonType === ComparisionType.Values ? valuesAgg : changeAgg,
         drop_missing_columns: false,

diff --git a/packages/superset-ui-chart-controls/src/types.ts b/packages/superset-ui-chart-controls/src/types.ts
@@ -19,11 +19,12 @@
  */
 import React, { ReactNode, ReactText, ReactElement } from 'react';
 import {
-  QueryFormData,
+  AdhocColumn,
+  Column,
   DatasourceType,
-  Metric,
   JsonValue,
-  Column,
+  Metric,
+  QueryFormData,
 } from '@superset-ui/core';
 import { sharedControls } from './shared-controls';
 import sharedControlComponents from './shared-controls/components';
@@ -380,3 +381,23 @@ export type ColorFormatters = {
 }[];
 
 export default {};
+
+export function isColumnMeta(
+  column: AdhocColumn | ColumnMeta,
+): column is ColumnMeta {
+  return 'column_name' in column;
+}
+
+export function isSavedExpression(
+  column: AdhocColumn | ColumnMeta,
+): column is ColumnMeta {
+  return (
+    'column_name' in column && 'expression' in column && !!column.expression
+  );
+}
+
+export function isAdhocColumn(
+  column: AdhocColumn | ColumnMeta,
+): column is AdhocColumn {
+  return 'label' in column && 'sqlExpression' in column;
+}
diff --git a/packages/superset-ui-chart-controls/test/types.test.ts b/packages/superset-ui-chart-controls/test/types.test.ts
@@ -0,0 +1,68 @@
+/**
+ * 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 { AdhocColumn } from '@superset-ui/core';
+import { isAdhocColumn, isColumnMeta, isSavedExpression } from '../src';
+import { ColumnMeta } from '../lib';
+
+const ADHOC_COLUMN: AdhocColumn = {
+  hasCustomLabel: true,
+  label: 'Adhoc column',
+  sqlExpression: 'case when 1 = 1 then 1 else 2 end',
+};
+const COLUMN_META: ColumnMeta = {
+  column_name: 'my_col',
+};
+const SAVED_EXPRESSION: ColumnMeta = {
+  column_name: 'Saved expression',
+  expression: 'case when 1 = 1 then 1 else 2 end',
+};
+
+describe('isColumnMeta', () => {
+  it('returns false for AdhocColumn', () => {
+    expect(isColumnMeta(ADHOC_COLUMN)).toEqual(false);
+  });
+
+  it('returns true for ColumnMeta', () => {
+    expect(isColumnMeta(COLUMN_META)).toEqual(true);
+  });
+});
+
+describe('isAdhocColumn', () => {
+  it('returns true for AdhocColumn', () => {
+    expect(isAdhocColumn(ADHOC_COLUMN)).toEqual(true);
+  });
+
+  it('returns false for ColumnMeta', () => {
+    expect(isAdhocColumn(COLUMN_META)).toEqual(false);
+  });
+});
+
+describe('isSavedExpression', () => {
+  it('returns false for AdhocColumn', () => {
+    expect(isSavedExpression(ADHOC_COLUMN)).toEqual(false);
+  });
+
+  it('returns false for ColumnMeta without expression', () => {
+    expect(isSavedExpression(COLUMN_META)).toEqual(false);
+  });
+
+  it('returns true for ColumnMeta with expression', () => {
+    expect(isSavedExpression(SAVED_EXPRESSION)).toEqual(true);
+  });
+});
diff --git a/packages/superset-ui-core/src/query/buildQueryObject.ts b/packages/superset-ui-core/src/query/buildQueryObject.ts
@@ -18,13 +18,17 @@
  */
 
 /* eslint-disable camelcase */
-import { QueryObject, QueryObjectFilterClause } from './types/Query';
-import { QueryFieldAliases, QueryFormData } from './types/QueryFormData';
+import {
+  AdhocFilter,
+  QueryFieldAliases,
+  QueryFormData,
+  QueryObject,
+  QueryObjectFilterClause,
+} from './types';
 import processFilters from './processFilters';
 import extractExtras from './extractExtras';
 import extractQueryFields from './extractQueryFields';
 import { overrideExtraFormData } from './processExtraFormData';
-import { AdhocFilter } from './types';
 
 /**
  * Build the common segments of all query objects (e.g. the granularity field derived from

diff --git a/packages/superset-ui-core/src/query/extractQueryFields.ts b/packages/superset-ui-core/src/query/extractQueryFields.ts
@@ -19,6 +19,7 @@
 import { t } from '../translation';
 import { removeDuplicates } from '../utils';
 import { DTTM_ALIAS } from './constants';
+import getColumnLabel from './getColumnLabel';
 import getMetricLabel from './getMetricLabel';
 import {
   QueryFields,
@@ -109,7 +110,10 @@ export default function extractQueryFields(
   }
 
   return {
-    columns: removeDuplicates(columns.filter(x => typeof x === 'string' && x)),
+    columns: removeDuplicates(
+      columns.filter(col => col !== ''),
+      getColumnLabel,
+    ),
     metrics:
       queryMode === QueryMode.raw
         ? undefined

diff --git a/packages/superset-ui-core/src/query/getColumnLabel.ts b/packages/superset-ui-core/src/query/getColumnLabel.ts
@@ -0,0 +1,11 @@
+import { isPhysicalColumn, QueryFormColumn } from './types';
+
+export default function getColumnLabel(column: QueryFormColumn): string {
+  if (isPhysicalColumn(column)) {
+    return column;
+  }
+  if (column.label) {
+    return column.label;
+  }
+  return column.sqlExpression;
+}
diff --git a/packages/superset-ui-core/src/query/getMetricLabel.ts b/packages/superset-ui-core/src/query/getMetricLabel.ts
@@ -17,16 +17,16 @@
  * under the License.
  */
 
-import { QueryFormMetric } from './types/QueryFormData';
+import { isAdhocMetricSimple, isSavedMetric, QueryFormMetric } from './types';
 
-export default function getMetricLabel(metric: QueryFormMetric) {
-  if (typeof metric === 'string') {
+export default function getMetricLabel(metric: QueryFormMetric): string {
+  if (isSavedMetric(metric)) {
     return metric;
   }
   if (metric.label) {
     return metric.label;
   }
-  if (metric.expressionType === 'SIMPLE') {
+  if (isAdhocMetricSimple(metric)) {
     return `${metric.aggregate}(${
       metric.column.columnName || metric.column.column_name
     })`;

diff --git a/packages/superset-ui-core/src/query/index.ts b/packages/superset-ui-core/src/query/index.ts
@@ -24,6 +24,7 @@ export { default as buildQueryContext } from './buildQueryContext';
 export { default as buildQueryObject } from './buildQueryObject';
 export { default as convertFilter } from './convertFilter';
 export { default as extractTimegrain } from './extractTimegrain';
+export { default as getColumnLabel } from './getColumnLabel';
 export { default as getMetricLabel } from './getMetricLabel';
 export { default as DatasourceKey } from './DatasourceKey';
 export { default as normalizeOrderBy } from './normalizeOrderBy';

diff --git a/packages/superset-ui-core/src/query/types/Column.ts b/packages/superset-ui-core/src/query/types/Column.ts
@@ -20,6 +20,19 @@
 
 import { GenericDataType } from './QueryResponse';
 
+export interface AdhocColumn {
+  hasCustomLabel?: boolean;
+  label?: string;
+  optionName?: string;
+  sqlExpression: string;
+  expressionType: 'SQL';
+}
+
+/**
+ * A column that is physically defined in datasource.
+ */
+export type PhysicalColumn = string;
+
 /**
  * Column information defined in datasource.
  */
@@ -39,3 +52,9 @@ export interface Column {
 }
 
 export default {};
+
+export function isPhysicalColumn(
+  column: AdhocColumn | PhysicalColumn,
+): column is PhysicalColumn {
+  return typeof column === 'string';
+}
diff --git a/packages/superset-ui-core/src/query/types/Metric.ts b/packages/superset-ui-core/src/query/types/Metric.ts
@@ -29,6 +29,7 @@ export type Aggregate =
   | 'SUM';
 
 export interface AdhocMetricBase {
+  hasCustomLabel?: boolean;
   label?: string;
   optionName?: string;
 }
@@ -72,3 +73,9 @@ export interface Metric {
 }
 
 export default {};
+
+export function isAdhocMetricSimple(
+  metric: AdhocMetric,
+): metric is AdhocMetricSimple {
+  return metric.expressionType === 'SIMPLE';
+}
diff --git a/packages/superset-ui-core/src/query/types/Query.ts b/packages/superset-ui-core/src/query/types/Query.ts
@@ -28,7 +28,7 @@ import { JsonObject } from '../../connection';
 import { TimeGranularity } from '../../time-format';
 
 export type QueryObjectFilterClause = {
-  col: string;
+  col: QueryFormColumn;
   grain?: TimeGranularity;
   isExtra?: boolean;
 } & (

diff --git a/packages/superset-ui-core/src/query/types/QueryFormData.ts b/packages/superset-ui-core/src/query/types/QueryFormData.ts
@@ -33,6 +33,7 @@ import {
 import { TimeRange, TimeRangeEndpoints } from './Time';
 import { TimeGranularity } from '../../time-format';
 import { JsonObject } from '../../connection';
+import { AdhocColumn, PhysicalColumn } from './Column';
 
 /**
  * Metric definition/reference in query object.
@@ -41,9 +42,9 @@ export type QueryFormMetric = SavedMetric | AdhocMetric;
 
 /**
  * Column selects in query object (used as dimensions in both groupby or raw
- * query mode). Only support referring existing columns.
+ * query mode). Can be either reference to physical column or expression.
  */
-export type QueryFormColumn = string;
+export type QueryFormColumn = PhysicalColumn | AdhocColumn;
 
 /**
  * Order query results by columns.
@@ -174,7 +175,7 @@ export interface BaseFormData extends TimeRange, FormDataResidual {
   /** row offset for server side pagination */
   row_offset?: string | number | null;
   /** The metric used to order timeseries for limiting */
-  timeseries_limit_metric?: QueryFormColumn;
+  timeseries_limit_metric?: QueryFormMetric;
   /** Force refresh */
   force?: boolean;
   result_format?: string;
@@ -222,4 +223,8 @@ export function isDruidFormData(
   return 'granularity' in formData;
 }
 
+export function isSavedMetric(metric: QueryFormMetric): metric is SavedMetric {
+  return typeof metric === 'string';
+}
+
 export default {};
diff --git a/packages/superset-ui-core/test/query/getColumnLabel.test.ts b/packages/superset-ui-core/test/query/getColumnLabel.test.ts
@@ -0,0 +1,42 @@
+/**
+ * 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 { getColumnLabel } from '@superset-ui/core/src/query';
+
+describe('getColumnLabel', () => {
+  it('should handle physical column', () => {
+    expect(getColumnLabel('gender')).toEqual('gender');
+  });
+
+  it('should handle adhoc columns with label', () => {
+    expect(
+      getColumnLabel({
+        sqlExpression: "case when 1 then 'a' else 'b' end",
+        label: 'my col',
+      }),
+    ).toEqual('my col');
+  });
+
+  it('should handle adhoc columns without label', () => {
+    expect(
+      getColumnLabel({
+        sqlExpression: "case when 1 then 'a' else 'b' end",
+      }),
+    ).toEqual("case when 1 then 'a' else 'b' end");
+  });
+});
diff --git a/packages/superset-ui-core/test/query/getMetricLabel.test.ts b/packages/superset-ui-core/test/query/getMetricLabel.test.ts
@@ -17,7 +17,6 @@
  * under the License.
  */
 import { getMetricLabel } from '@superset-ui/core/src/query';
-import { GenericDataType } from '../../src';
 
 describe('getMetricLabel', () => {
   it('should handle predefined metric name', () => {