feat: add generic type to column payload (#14547)

* feat: add generic type to column payload * feat: add generic type to column payload * xit flaky test
apache · May 13, 2021 · 3f6bd1e · 3f6bd1e
1 parent ad699e8
commit 3f6bd1e
Show file tree

Hide file tree

Showing 15 changed files with 466 additions and 357 deletions.
diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts
@@ -45,7 +45,9 @@ export interface ChartSpec {
 
 export function getChartGridComponent({ name, viz }: ChartSpec) {
   return cy
-    .get(`[data-test="chart-grid-component"][data-test-chart-name="${name}"]`)
+    .get(`[data-test="chart-grid-component"][data-test-chart-name="${name}"]`, {
+      timeout: 30000,
+    })
     .should('have.attr', 'data-test-viz-type', viz);
 }
 

diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/load.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/load.test.ts
@@ -44,7 +44,9 @@ describe('Dashboard load', () => {
 
   it('should load in edit/standalone mode', () => {
     cy.visit(`${WORLD_HEALTH_DASHBOARD}?edit=true&standalone=true`);
-    cy.get('[data-test="discard-changes-button"]').should('be.visible');
+    cy.get('[data-test="discard-changes-button"]', { timeout: 10000 }).should(
+      'be.visible',
+    );
     cy.get('#app-menu').should('not.exist');
   });
 

diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git a/superset-frontend/package.json b/superset-frontend/package.json
@@ -67,35 +67,35 @@
     "@emotion/babel-preset-css-prop": "^11.2.0",
     "@emotion/cache": "^11.1.3",
     "@emotion/react": "^11.1.5",
-    "@superset-ui/chart-controls": "^0.17.42",
-    "@superset-ui/core": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-calendar": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-chord": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-country-map": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-event-flow": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-force-directed": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-heatmap": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-histogram": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-horizon": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-map-box": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-partition": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-rose": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-sankey": "^0.17.43",
-    "@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-sunburst": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-treemap": "^0.17.42",
-    "@superset-ui/legacy-plugin-chart-world-map": "^0.17.42",
-    "@superset-ui/legacy-preset-chart-big-number": "^0.17.42",
+    "@superset-ui/chart-controls": "^0.17.46",
+    "@superset-ui/core": "^0.17.46",
+    "@superset-ui/legacy-plugin-chart-calendar": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-chord": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-country-map": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-event-flow": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-force-directed": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-heatmap": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-histogram": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-horizon": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-map-box": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-paired-t-test": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-parallel-coordinates": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-partition": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-pivot-table": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-rose": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-sankey": "^0.17.46",
+    "@superset-ui/legacy-plugin-chart-sankey-loop": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-sunburst": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-treemap": "0.17.46",
+    "@superset-ui/legacy-plugin-chart-world-map": "0.17.46",
+    "@superset-ui/legacy-preset-chart-big-number": "0.17.46",
     "@superset-ui/legacy-preset-chart-deckgl": "^0.4.6",
-    "@superset-ui/legacy-preset-chart-nvd3": "^0.17.42",
-    "@superset-ui/plugin-chart-echarts": "^0.17.44",
-    "@superset-ui/plugin-chart-pivot-table": "^0.17.44",
-    "@superset-ui/plugin-chart-table": "^0.17.44",
-    "@superset-ui/plugin-chart-word-cloud": "^0.17.42",
-    "@superset-ui/preset-chart-xy": "^0.17.42",
+    "@superset-ui/legacy-preset-chart-nvd3": "0.17.46",
+    "@superset-ui/plugin-chart-echarts": "^0.17.46",
+    "@superset-ui/plugin-chart-pivot-table": "^0.17.46",
+    "@superset-ui/plugin-chart-table": "^0.17.46",
+    "@superset-ui/plugin-chart-word-cloud": "0.17.46",
+    "@superset-ui/preset-chart-xy": "0.17.46",
     "@vx/responsive": "^0.0.195",
     "abortcontroller-polyfill": "^1.1.9",
     "antd": "^4.9.4",

diff --git a/superset-frontend/spec/javascripts/components/ColumnTypeLabel_spec.jsx b/superset-frontend/spec/javascripts/components/ColumnTypeLabel_spec.jsx
@@ -20,10 +20,11 @@ import React from 'react';
 import { shallow } from 'enzyme';
 
 import { ColumnTypeLabel } from '@superset-ui/chart-controls';
+import { GenericDataType } from '@superset-ui/core';
 
 describe('ColumnOption', () => {
   const defaultProps = {
-    type: 'string',
+    type: GenericDataType.STRING,
   };
 
   const props = { ...defaultProps };
@@ -44,12 +45,16 @@ describe('ColumnOption', () => {
     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');
   });
@@ -64,7 +69,9 @@ describe('ColumnOption', () => {
     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);
   });
 });
diff --git a/superset-frontend/spec/javascripts/datasource/fixtures.tsx b/superset-frontend/spec/javascripts/datasource/fixtures.tsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 import { ColumnMeta } from '@superset-ui/chart-controls';
-import { ColumnType } from '@superset-ui/core';
+import { GenericDataType } from '@superset-ui/core';
 
 export const columns: ColumnMeta[] = [
   {
@@ -29,7 +29,8 @@ export const columns: ColumnMeta[] = [
     id: 516,
     is_dttm: false,
     python_date_format: null,
-    type: ColumnType.DOUBLE,
+    type: 'DOUBLE',
+    type_generic: GenericDataType.NUMERIC,
     verbose_name: null,
   },
   {
@@ -42,7 +43,8 @@ export const columns: ColumnMeta[] = [
     id: 477,
     is_dttm: false,
     python_date_format: null,
-    type: ColumnType.STRING,
+    type: 'VARCHAR',
+    type_generic: GenericDataType.STRING,
     verbose_name: null,
   },
   {
@@ -54,7 +56,8 @@ export const columns: ColumnMeta[] = [
     id: 516,
     is_dttm: false,
     python_date_format: null,
-    type: ColumnType.DOUBLE,
+    type: 'INT',
+    type_generic: GenericDataType.NUMERIC,
     verbose_name: null,
   },
 ];

diff --git a/...-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx b/...-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx
@@ -17,13 +17,13 @@
  * under the License.
  */
 import React from 'react';
-import { ColumnType } from '@superset-ui/core';
 import { render, screen } from 'spec/helpers/testing-library';
 import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
 import AdhocFilter, {
   EXPRESSION_TYPES,
 } from 'src/explore/components/controls/FilterControl/AdhocFilter';
 import { DndFilterSelect } from 'src/explore/components/controls/DndColumnSelectControl/DndFilterSelect';
+import { GenericDataType } from '@superset-ui/core';
 
 const defaultProps = {
   name: 'Filter',
@@ -63,7 +63,14 @@ test('renders options with column', () => {
   render(
     <DndFilterSelect
       {...defaultProps}
-      columns={[{ id: 1, type: ColumnType.STRING, column_name: 'Column' }]}
+      columns={[
+        {
+          id: 1,
+          type: 'VARCHAR',
+          type_generic: GenericDataType.STRING,
+          column_name: 'Column',
+        },
+      ]}
     />,
     {
       useDnd: true,

diff --git a/superset-frontend/src/filters/components/Range/buildQuery.ts b/superset-frontend/src/filters/components/Range/buildQuery.ts
@@ -18,7 +18,7 @@
  */
 import {
   buildQueryContext,
-  ColumnType,
+  GenericDataType,
   QueryFormData,
 } from '@superset-ui/core';
 
@@ -52,7 +52,7 @@ export default function buildQuery(formData: QueryFormData) {
           column: {
             column_name: column,
             id: 1,
-            type: ColumnType.FLOAT,
+            type_generic: GenericDataType.NUMERIC,
           },
           expressionType: 'SIMPLE',
           hasCustomLabel: true,
@@ -63,7 +63,7 @@ export default function buildQuery(formData: QueryFormData) {
           column: {
             column_name: column,
             id: 2,
-            type: ColumnType.FLOAT,
+            type_generic: GenericDataType.NUMERIC,
           },
           expressionType: 'SIMPLE',
           hasCustomLabel: true,

diff --git a/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx b/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx
@@ -16,7 +16,13 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { ensureIsArray, ExtraFormData, t, tn } from '@superset-ui/core';
+import {
+  ensureIsArray,
+  ExtraFormData,
+  t,
+  TimeGranularity,
+  tn,
+} from '@superset-ui/core';
 import React, { useEffect, useState } from 'react';
 import { Select } from 'src/common/components';
 import { Styles, StyledSelect } from '../common';
@@ -38,7 +44,7 @@ export default function PluginFilterTimegrain(
 
     const extraFormData: ExtraFormData = {};
     if (timeGrain) {
-      extraFormData.time_grain_sqla = timeGrain;
+      extraFormData.time_grain_sqla = timeGrain as TimeGranularity;
     }
     setValue(resultValue);
     setDataMask({

diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py
@@ -533,6 +533,7 @@ class BaseColumn(AuditMixinNullable, ImportExportMixin):
     def __repr__(self) -> str:
         return str(self.column_name)
 
+    bool_types = ("BOOL",)
     num_types = (
         "DOUBLE",
         "FLOAT",
@@ -560,6 +561,22 @@ def is_temporal(self) -> bool:
     def is_string(self) -> bool:
         return self.type and any(map(lambda t: t in self.type.upper(), self.str_types))
 
+    @property
+    def is_boolean(self) -> bool:
+        return self.type and any(map(lambda t: t in self.type.upper(), self.bool_types))
+
+    @property
+    def type_generic(self) -> Optional[utils.GenericDataType]:
+        if self.is_string:
+            return utils.GenericDataType.STRING
+        if self.is_boolean:
+            return utils.GenericDataType.BOOLEAN
+        if self.is_numeric:
+            return utils.GenericDataType.NUMERIC
+        if self.is_temporal:
+            return utils.GenericDataType.TEMPORAL
+        return None
+
     @property
     def expression(self) -> Column:
         raise NotImplementedError()

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
@@ -191,6 +191,16 @@ class TableColumn(Model, BaseColumn):
     update_from_object_fields = [s for s in export_fields if s not in ("table_id",)]
     export_parent = "table"
 
+    @property
+    def is_boolean(self) -> bool:
+        """
+        Check if the column has a boolean datatype.
+        """
+        column_spec = self.table.database.db_engine_spec.get_column_spec(self.type)
+        if column_spec is None:
+            return False
+        return column_spec.generic_type == GenericDataType.BOOLEAN
+
     @property
     def is_numeric(self) -> bool:
         """
@@ -349,6 +359,7 @@ def data(self) -> Dict[str, Any]:
             "groupby",
             "is_dttm",
             "type",
+            "type_generic",
             "python_date_format",
         )
         return {s: getattr(self, s) for s in attrs if hasattr(self, s)}

diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
@@ -194,6 +194,12 @@ class BaseEngineSpec:  # pylint: disable=too-many-public-methods
             types.Numeric(),
             GenericDataType.NUMERIC,
         ),
+        (re.compile(r"^float", re.IGNORECASE), types.Float(), GenericDataType.NUMERIC,),
+        (
+            re.compile(r"^double", re.IGNORECASE),
+            types.Float(),
+            GenericDataType.NUMERIC,
+        ),
         (re.compile(r"^real", re.IGNORECASE), types.REAL, GenericDataType.NUMERIC,),
         (
             re.compile(r"^smallserial", re.IGNORECASE),
@@ -210,6 +216,11 @@ class BaseEngineSpec:  # pylint: disable=too-many-public-methods
             types.BigInteger(),
             GenericDataType.NUMERIC,
         ),
+        (
+            re.compile(r"^money", re.IGNORECASE),
+            types.Numeric(),
+            GenericDataType.NUMERIC,
+        ),
         (
             re.compile(r"^string", re.IGNORECASE),
             types.String(),
@@ -225,6 +236,12 @@ class BaseEngineSpec:  # pylint: disable=too-many-public-methods
             String(),
             utils.GenericDataType.STRING,
         ),
+        (
+            re.compile(r"^((TINY|MEDIUM|LONG)?TEXT)", re.IGNORECASE),
+            String(),
+            utils.GenericDataType.STRING,
+        ),
+        (re.compile(r"^LONG", re.IGNORECASE), types.Float(), GenericDataType.NUMERIC,),
         (
             re.compile(r"^datetime", re.IGNORECASE),
             types.DateTime(),

diff --git a/tests/db_engine_specs/base_tests.py b/tests/db_engine_specs/base_tests.py
@@ -16,22 +16,37 @@
 # under the License.
 # isort:skip_file
 from datetime import datetime
+from typing import Tuple, Type
 
 from tests.test_app import app
 from tests.base_tests import SupersetTestCase
-from superset.db_engine_specs.mysql import MySQLEngineSpec
+from superset.db_engine_specs.base import BaseEngineSpec
 from superset.models.core import Database
+from superset.utils.core import GenericDataType
 
 
 class TestDbEngineSpec(SupersetTestCase):
     def sql_limit_regex(
         self,
         sql,
         expected_sql,
-        engine_spec_class=MySQLEngineSpec,
+        engine_spec_class=BaseEngineSpec,
         limit=1000,
         force=False,
     ):
         main = Database(database_name="test_database", sqlalchemy_uri="sqlite://")
         limited = engine_spec_class.apply_limit_to_sql(sql, limit, main, force)
         self.assertEqual(expected_sql, limited)
+
+
+def assert_generic_types(
+    spec: Type[BaseEngineSpec],
+    type_expectations: Tuple[Tuple[str, GenericDataType], ...],
+) -> None:
+    for type_str, expected_type in type_expectations:
+        column_spec = spec.get_column_spec(type_str)
+        assert column_spec is not None
+        actual_type = column_spec.generic_type
+        assert (
+            actual_type == expected_type
+        ), f"{type_str} should be {expected_type.name} but is {actual_type.name}"