fix(core): clean up datatype logic (#1102)

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx

@@ -18,7 +18,7 @@
  */
 import React from 'react';
 import { Tooltip } from './Tooltip';
-import { ColumnTypeLabel, LaxColumnType } from './ColumnTypeLabel';
+import { ColumnTypeLabel } from './ColumnTypeLabel';
 import InfoTooltipWithTrigger from './InfoTooltipWithTrigger';
 import { ColumnMeta } from '../types';
 
@@ -28,18 +28,13 @@ export type ColumnOptionProps = {
 };
 
 export function ColumnOption({ column, showType = false }: ColumnOptionProps) {
-  const hasExpression = column.expression && column.expression !== column.column_name;
-
-  let columnType: LaxColumnType | undefined = column.type;
-  if (column.is_dttm) {
-    columnType = 'time';
-  } else if (hasExpression) {
-    columnType = 'expression';
-  }
+  const { expression, column_name, type_generic } = column;
+  const hasExpression = expression && expression !== column_name;
+  const type = hasExpression ? 'expression' : type_generic;
 
   return (
     <span>
-      {showType && columnType && <ColumnTypeLabel type={columnType} />}
+      {showType && type !== undefined && <ColumnTypeLabel type={type} />}
       <Tooltip
         id="metric-name-tooltip"
         title={column.verbose_name || column.column_name}

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/components/ColumnTypeLabel.tsx

@@ -17,47 +17,32 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { ColumnType, GenericDataType } from '@superset-ui/core';
+import { GenericDataType } from '@superset-ui/core';
 import React from 'react';
 
-export type LaxColumnType = ColumnType | GenericDataType | 'expression' | 'aggregate' | 'time' | '';
+type StringIcon = '?' | 'ƒ' | 'AGG' | 'ABC' | '#' | 'T/F' | 'time';
+
+export type ColumnLabelExtendedType = 'expression' | 'aggregate' | '';
 
 export type ColumnTypeLabelProps = {
-  type?: LaxColumnType;
+  type?: ColumnLabelExtendedType | GenericDataType;
 };
 
-export function ColumnTypeLabel({ type: type_ }: ColumnTypeLabelProps) {
-  const type: string =
-    type_ === undefined || type_ === null
-      ? '?'
-      : type_ === GenericDataType.BOOLEAN
-      ? 'bool'
-      : type_ === GenericDataType.NUMERIC
-      ? 'FLOAT'
-      : type_ === GenericDataType.TEMPORAL
-      ? 'time'
-      : type_ === GenericDataType.STRING
-      ? 'string'
-      : type_;
+export function ColumnTypeLabel({ type }: ColumnTypeLabelProps) {
+  let stringIcon: StringIcon = '?';
 
-  let stringIcon;
-
-  if (typeof type !== 'string') {
-    stringIcon = '?';
-  } else if (type === '' || type === 'expression') {
+  if (type === '' || type === 'expression') {
     stringIcon = 'ƒ';
   } else if (type === 'aggregate') {
     stringIcon = 'AGG';
-  } else if (type.match(/.*char.*/i) || type.match(/string.*/i) || type.match(/.*text.*/i)) {
+  } else if (type === GenericDataType.STRING) {
     stringIcon = 'ABC';
-  } else if (type.match(/.*int.*/i) || type === 'LONG' || type === 'DOUBLE' || type === 'FLOAT') {
+  } else if (type === GenericDataType.NUMERIC) {
     stringIcon = '#';
-  } else if (type.match(/.*bool.*/i)) {
+  } else if (type === GenericDataType.BOOLEAN) {
     stringIcon = 'T/F';
-  } else if (type.match(/.*time.*/i)) {
+  } else if (type === GenericDataType.TEMPORAL) {
     stringIcon = 'time';
-  } else {
-    stringIcon = '?';
   }
 
   const typeIcon =

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/constants.ts

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { t, QueryMode, DTTM_ALIAS } from '@superset-ui/core';
+import { t, QueryMode, DTTM_ALIAS, GenericDataType } from '@superset-ui/core';
 import { ColumnMeta } from './types';
 
 // eslint-disable-next-line import/prefer-default-export
@@ -35,6 +35,7 @@ export const COLUMN_NAME_ALIASES: Record<string, string> = {
 export const TIME_COLUMN_OPTION: ColumnMeta = {
   verbose_name: COLUMN_NAME_ALIASES[DTTM_ALIAS],
   column_name: DTTM_ALIAS,
+  type_generic: GenericDataType.TEMPORAL,
   description: t('A reference to the [Time] configuration, taking granularity into account'),
 };
 

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/types.ts

@@ -18,14 +18,7 @@
  * under the License.
  */
 import React, { ReactNode, ReactText, ReactElement } from 'react';
-import {
-  QueryFormData,
-  DatasourceType,
-  Metric,
-  JsonValue,
-  Column,
-  ColumnType,
-} 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';
 
@@ -45,9 +38,8 @@ export type SharedControlComponents = typeof sharedControlComponents;
 /** ----------------------------------------------
  * Input data/props while rendering
  * ---------------------------------------------*/
-export type ColumnMeta = Omit<Column, 'id' | 'type'> & {
+export type ColumnMeta = Omit<Column, 'id'> & {
   id?: number;
-  type?: ColumnType;
 } & AnyDict;
 
 export interface DatasourceMeta {

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx

@@ -18,6 +18,7 @@
  */
 import React from 'react';
 import { shallow, ShallowWrapper } from 'enzyme';
+import { GenericDataType } from '@superset-ui/core';
 
 import {
   ColumnOption,
@@ -27,7 +28,7 @@ import {
 } from '../../src';
 
 describe('ColumnOption', () => {
-  const defaultProps = {
+  const defaultProps: ColumnOptionProps = {
     column: {
       column_name: 'foo',
       verbose_name: 'Foo',
@@ -72,7 +73,8 @@ describe('ColumnOption', () => {
         showType: true,
         column: {
           column_name: 'foo',
-          type: 'str',
+          type: 'VARCHAR',
+          type_generic: GenericDataType.STRING,
         },
       }),
     );
@@ -98,9 +100,10 @@ describe('ColumnOption', () => {
   });
   it('dttm column has correct column label if showType is true', () => {
     props.showType = true;
-    props.column.is_dttm = true;
+    props.column.expression = undefined;
+    props.column.type_generic = GenericDataType.TEMPORAL;
     wrapper = shallow(factory(props));
     expect(wrapper.find(ColumnTypeLabel)).toHaveLength(1);
-    expect(wrapper.find(ColumnTypeLabel).props().type).toBe('time');
+    expect(wrapper.find(ColumnTypeLabel).props().type).toBe(GenericDataType.TEMPORAL);
   });
 });

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/test/components/ColumnTypeLabel.test.tsx

@@ -18,12 +18,13 @@
  */
 import React from 'react';
 import { shallow } from 'enzyme';
+import { GenericDataType } from '@superset-ui/core';
 
 import { ColumnTypeLabel, ColumnTypeLabelProps } from '../../src';
 
 describe('ColumnOption', () => {
   const defaultProps = {
-    type: 'string',
+    type: GenericDataType.STRING,
   };
 
   const props = { ...defaultProps };
@@ -37,17 +38,17 @@ describe('ColumnOption', () => {
     expect(React.isValidElement(<ColumnTypeLabel {...defaultProps} />)).toBe(true);
   });
   it('string type shows ABC icon', () => {
-    const lbl = getWrapper({}).find('.type-label');
+    const lbl = getWrapper({ type: GenericDataType.STRING }).find('.type-label');
     expect(lbl).toHaveLength(1);
     expect(lbl.first().text()).toBe('ABC');
   });
   it('int type shows # icon', () => {
-    const lbl = getWrapper({ type: 'int(164)' }).find('.type-label');
+    const lbl = getWrapper({ type: GenericDataType.NUMERIC }).find('.type-label');
     expect(lbl).toHaveLength(1);
     expect(lbl.first().text()).toBe('#');
   });
   it('bool type shows T/F icon', () => {
-    const lbl = getWrapper({ type: 'BOOL' }).find('.type-label');
+    const lbl = getWrapper({ type: GenericDataType.BOOLEAN }).find('.type-label');
     expect(lbl).toHaveLength(1);
     expect(lbl.first().text()).toBe('T/F');
   });
@@ -57,12 +58,12 @@ describe('ColumnOption', () => {
     expect(lbl.first().text()).toBe('ƒ');
   });
   it('unknown type shows question mark', () => {
-    const lbl = getWrapper({ type: 'unknown' }).find('.type-label');
+    const lbl = getWrapper({ type: undefined }).find('.type-label');
     expect(lbl).toHaveLength(1);
     expect(lbl.first().text()).toBe('?');
   });
   it('datetime type displays', () => {
-    const lbl = getWrapper({ type: 'datetime' }).find('.fa-clock-o');
+    const lbl = getWrapper({ type: GenericDataType.TEMPORAL }).find('.fa-clock-o');
     expect(lbl).toHaveLength(1);
   });
 });

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/types/Column.ts

@@ -17,30 +17,16 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-export enum ColumnType {
-  DOUBLE = 'DOUBLE',
-  FLOAT = 'FLOAT',
-  INT = 'INT',
-  BIGINT = 'BIGINT',
-  LONG = 'LONG',
-  REAL = 'REAL',
-  NUMERIC = 'NUMERIC',
-  DECIMAL = 'DECIMAL',
-  MONEY = 'MONEY',
-  DATE = 'DATE',
-  TIME = 'TIME',
-  DATETIME = 'DATETIME',
-  VARCHAR = 'VARCHAR',
-  STRING = 'STRING',
-  CHAR = 'CHAR',
-}
+
+import { GenericDataType } from './QueryResponse';
 
 /**
  * Column information defined in datasource.
  */
 export interface Column {
   id: number;
-  type: ColumnType;
+  type?: string;
+  type_generic?: GenericDataType;
   column_name: string;
   groupby?: boolean;
   is_dttm?: boolean;

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/fixtures.ts

@@ -16,14 +16,15 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { AdhocMetric, ColumnType } from '../src';
+import { AdhocMetric, GenericDataType } from '../src';
 
 export const NUM_METRIC: AdhocMetric = {
   expressionType: 'SIMPLE',
   label: 'Sum(num)',
   column: {
     id: 336,
-    type: ColumnType.BIGINT,
+    type: 'BIGINT',
+    type_generic: GenericDataType.NUMERIC,
     column_name: 'num',
     verbose_name: null,
     description: null,

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/query/getMetricLabel.test.ts

@@ -16,7 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { ColumnType, getMetricLabel } from '@superset-ui/core/src/query';
+import { getMetricLabel } from '@superset-ui/core/src/query';
+import { GenericDataType } from '../../src';
 
 describe('getMetricLabel', () => {
   it('should handle predefined metric name', () => {
@@ -30,7 +31,7 @@ describe('getMetricLabel', () => {
         aggregate: 'AVG',
         column: {
           id: 5,
-          type: ColumnType.BIGINT,
+          type: 'BIGINT',
           columnName: 'sum_girls',
         },
       }),
@@ -44,7 +45,7 @@ describe('getMetricLabel', () => {
         aggregate: 'AVG',
         column: {
           id: 5,
-          type: ColumnType.BIGINT,
+          type: 'BIGINT',
           column_name: 'sum_girls',
         },
       }),