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
2021-05-13 09:36:09 +03:00 · 2021-05-13 09:36:09 +03:00 · 3f6bd1e4a4
parent ad699e8b48
commit 3f6bd1e4a4
15 changed files with 466 additions and 357 deletions
--- 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);
 }

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

--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
--- 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",
--- 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);
  });
 });
--- 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,
  },
 ];
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx
+++ b/superset-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,
--- 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,
--- 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({
--- 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 @@ class BaseColumn(AuditMixinNullable, ImportExportMixin):
    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()
--- 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 @@ class TableColumn(Model, BaseColumn):
            "groupby",
            "is_dttm",
            "type",
+            "type_generic",
            "python_date_format",
        )
        return {s: getattr(self, s) for s in attrs if hasattr(self, s)}
--- 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(),
--- a/tests/db_engine_specs/base_tests.py
+++ b/tests/db_engine_specs/base_tests.py
@ -16,11 +16,13 @@
 # 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):
@ -28,10 +30,23 @@ class TestDbEngineSpec(SupersetTestCase):
        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}"
--- a/tests/db_engine_specs/mysql_tests.py
+++ b/tests/db_engine_specs/mysql_tests.py
@ -22,7 +22,7 @@ from sqlalchemy.dialects.mysql import DATE, NVARCHAR, TEXT, VARCHAR
 from superset.db_engine_specs.mysql import MySQLEngineSpec
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.utils.core import GenericDataType
-from tests.db_engine_specs.base_tests import TestDbEngineSpec
+from tests.db_engine_specs.base_tests import assert_generic_types, TestDbEngineSpec


 class TestMySQLEngineSpecsDbEngineSpec(TestDbEngineSpec):
@ -65,7 +65,7 @@ class TestMySQLEngineSpecsDbEngineSpec(TestDbEngineSpec):
            )
            self.assertEqual(actual, expected)

-    def test_is_db_column_type_match(self):
+    def test_generic_type(self):
        type_expectations = (
            # Numeric
            ("TINYINT", GenericDataType.NUMERIC),
@ -89,10 +89,7 @@ class TestMySQLEngineSpecsDbEngineSpec(TestDbEngineSpec):
            ("TIMESTAMP", GenericDataType.TEMPORAL),
            ("TIME", GenericDataType.TEMPORAL),
        )
-
-        for type_str, col_type in type_expectations:
-            column_spec = MySQLEngineSpec.get_column_spec(type_str)
-            assert column_spec.generic_type == col_type
+        assert_generic_types(MySQLEngineSpec, type_expectations)

    def test_extract_error_message(self):
        from MySQLdb._exceptions import OperationalError
--- a/tests/db_engine_specs/postgres_tests.py
+++ b/tests/db_engine_specs/postgres_tests.py
@ -23,8 +23,8 @@ from sqlalchemy.dialects import postgresql
 from superset.db_engine_specs import get_engine_specs
 from superset.db_engine_specs.postgres import PostgresEngineSpec
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
-from superset.utils.core import get_example_database
-from tests.db_engine_specs.base_tests import TestDbEngineSpec
+from superset.utils.core import GenericDataType
+from tests.db_engine_specs.base_tests import assert_generic_types, TestDbEngineSpec
 from tests.fixtures.certificates import ssl_certificate
 from tests.fixtures.database import default_db_extra

@ -461,3 +461,28 @@ def test_base_parameters_mixin():
        },
        "required": ["database", "host", "port", "username"],
    }
+
+
+def test_generic_type():
+    type_expectations = (
+        # Numeric
+        ("SMALLINT", GenericDataType.NUMERIC),
+        ("INTEGER", GenericDataType.NUMERIC),
+        ("BIGINT", GenericDataType.NUMERIC),
+        ("DECIMAL", GenericDataType.NUMERIC),
+        ("NUMERIC", GenericDataType.NUMERIC),
+        ("REAL", GenericDataType.NUMERIC),
+        ("DOUBLE PRECISION", GenericDataType.NUMERIC),
+        ("MONEY", GenericDataType.NUMERIC),
+        # String
+        ("CHAR", GenericDataType.STRING),
+        ("VARCHAR", GenericDataType.STRING),
+        ("TEXT", GenericDataType.STRING),
+        # Temporal
+        ("DATE", GenericDataType.TEMPORAL),
+        ("TIMESTAMP", GenericDataType.TEMPORAL),
+        ("TIME", GenericDataType.TEMPORAL),
+        # Boolean
+        ("BOOLEAN", GenericDataType.BOOLEAN),
+    )
+    assert_generic_types(PostgresEngineSpec, type_expectations)