This is an automated email from the ASF dual-hosted git repository. yjc pushed a commit to branch table-chart-new-api in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit a1e87a8c107764ee1d53b5e5d37eda453182cf2a Author: Jesse Yang <[email protected]> AuthorDate: Thu Jul 9 09:26:58 2020 -0700 refactor: migrate table chart to new API --- .../integration/dashboard/url_params.test.js | 3 +- .../spec/javascripts/explore/controlUtils_spec.jsx | 12 ------- .../spec/javascripts/explore/store_spec.jsx | 2 -- superset-frontend/src/explore/controlUtils.js | 5 +-- superset-frontend/src/explore/store.js | 3 -- superset/common/query_context.py | 2 ++ superset/connectors/sqla/models.py | 6 ++-- superset/db_engine_specs/base.py | 32 +++++++---------- superset/result_set.py | 4 +-- superset/utils/core.py | 20 +++++++++-- tests/sqla_models_tests.py | 42 +++++++++++----------- 11 files changed, 60 insertions(+), 71 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/url_params.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/url_params.test.js index ae98be1..4300a26 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/url_params.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/url_params.test.js @@ -36,7 +36,7 @@ describe('Dashboard form data', () => { }); }); - it('should apply url params and queryFields to slice requests', () => { + it('should apply url params to slice requests', () => { const aliases = getChartAliases(dashboard.slices); // wait and verify one-by-one cy.wait(aliases).then(requests => { @@ -48,7 +48,6 @@ describe('Dashboard form data', () => { if (isLegacyResponse(responseBody)) { const requestFormData = xhr.request.body; const requestParams = JSON.parse(requestFormData.get('form_data')); - expect(requestParams).to.have.property('queryFields'); expect(requestParams.url_params).deep.eq(urlParams); } else { xhr.request.body.queries.forEach(query => { diff --git a/superset-frontend/spec/javascripts/explore/controlUtils_spec.jsx b/superset-frontend/spec/javascripts/explore/controlUtils_spec.jsx index e52f70d..137e670 100644 --- a/superset-frontend/spec/javascripts/explore/controlUtils_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/controlUtils_spec.jsx @@ -198,18 +198,6 @@ describe('controlUtils', () => { }); }); - describe('queryFields', () => { - it('in formData', () => { - const controlsState = getAllControlsState('table', 'table', {}, {}); - const formData = getFormDataFromControls(controlsState); - expect(formData.queryFields).toEqual({ - all_columns: 'columns', - metric: 'metrics', - metrics: 'metrics', - }); - }); - }); - describe('findControlItem', () => { it('find control as a string', () => { const controlItem = findControlItem( diff --git a/superset-frontend/spec/javascripts/explore/store_spec.jsx b/superset-frontend/spec/javascripts/explore/store_spec.jsx index 56293f0..c6fb98b 100644 --- a/superset-frontend/spec/javascripts/explore/store_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/store_spec.jsx @@ -72,12 +72,10 @@ describe('store', () => { const inputFormData = { datasource: '11_table', viz_type: 'table', - queryFields: staleQueryFields, this_should_no_be_here: true, }; const outputFormData = applyDefaultFormData(inputFormData); expect(outputFormData.this_should_no_be_here).toBe(undefined); - expect(outputFormData.queryFields).not.toBe(staleQueryFields); }); }); }); diff --git a/superset-frontend/src/explore/controlUtils.js b/superset-frontend/src/explore/controlUtils.js index c05e4de..7b83ea6 100644 --- a/superset-frontend/src/explore/controlUtils.js +++ b/superset-frontend/src/explore/controlUtils.js @@ -22,13 +22,10 @@ import { expandControlConfig } from '@superset-ui/chart-controls'; import * as SECTIONS from './controlPanels/sections'; export function getFormDataFromControls(controlsState) { - const formData = { queryFields: {} }; + const formData = {}; Object.keys(controlsState).forEach(controlName => { const control = controlsState[controlName]; formData[controlName] = control.value; - if (control.hasOwnProperty('queryField')) { - formData.queryFields[controlName] = control.queryField; - } }); return formData; } diff --git a/superset-frontend/src/explore/store.js b/superset-frontend/src/explore/store.js index 083ed58..bfef052 100644 --- a/superset-frontend/src/explore/store.js +++ b/superset-frontend/src/explore/store.js @@ -76,9 +76,6 @@ export function applyDefaultFormData(inputFormData) { formData[controlName] = inputFormData[controlName]; } }); - - // always use dynamically generated queryFields - formData.queryFields = controlFormData.queryFields; return formData; } diff --git a/superset/common/query_context.py b/superset/common/query_context.py index 19e6668..e203e75 100644 --- a/superset/common/query_context.py +++ b/superset/common/query_context.py @@ -162,6 +162,8 @@ class QueryContext: df = payload["df"] status = payload["status"] if status != utils.QueryStatus.FAILED: + payload["columns"] = list(df.columns) + payload["column_types"] = utils.serialize_dtypes(df.dtypes) payload["data"] = self.get_data(df) del payload["df"] diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index e19c1b9..f4f099a 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -189,7 +189,7 @@ class TableColumn(Model, BaseColumn): """ db_engine_spec = self.table.database.db_engine_spec return db_engine_spec.is_db_column_type_match( - self.type, utils.DbColumnType.NUMERIC + self.type, utils.GenericColumnType.NUMERIC ) @property @@ -199,7 +199,7 @@ class TableColumn(Model, BaseColumn): """ db_engine_spec = self.table.database.db_engine_spec return db_engine_spec.is_db_column_type_match( - self.type, utils.DbColumnType.STRING + self.type, utils.GenericColumnType.STRING ) @property @@ -214,7 +214,7 @@ class TableColumn(Model, BaseColumn): return self.is_dttm db_engine_spec = self.table.database.db_engine_spec return db_engine_spec.is_db_column_type_match( - self.type, utils.DbColumnType.TEMPORAL + self.type, utils.GenericColumnType.TEMPORAL ) def get_sqla_col(self, label: Optional[str] = None) -> Column: diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 0aee290..e86a676 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -157,34 +157,28 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods max_column_name_length = 0 try_remove_schema_from_table_name = True # pylint: disable=invalid-name - # default matching patterns for identifying column types - db_column_types: Dict[utils.DbColumnType, Tuple[Pattern[Any], ...]] = { - utils.DbColumnType.NUMERIC: ( + # default matching patterns to convert database specific column types to + # more generic types + db_column_types: Dict[utils.GenericColumnType, Tuple[Pattern[Any], ...]] = { + utils.GenericColumnType.NUMERIC: ( re.compile(r"BIT", re.IGNORECASE), - re.compile(r".*DOUBLE.*", re.IGNORECASE), - re.compile(r".*FLOAT.*", re.IGNORECASE), - re.compile(r".*INT.*", re.IGNORECASE), - re.compile(r".*NUMBER.*", re.IGNORECASE), + re.compile( + r".*(DOUBLE|FLOAT|INT|NUMBER|REAL|NUMERIC|DECIMAL|MONEY).*", + re.IGNORECASE, + ), re.compile(r".*LONG$", re.IGNORECASE), - re.compile(r".*REAL.*", re.IGNORECASE), - re.compile(r".*NUMERIC.*", re.IGNORECASE), - re.compile(r".*DECIMAL.*", re.IGNORECASE), - re.compile(r".*MONEY.*", re.IGNORECASE), ), - utils.DbColumnType.STRING: ( - re.compile(r".*CHAR.*", re.IGNORECASE), - re.compile(r".*STRING.*", re.IGNORECASE), - re.compile(r".*TEXT.*", re.IGNORECASE), + utils.GenericColumnType.STRING: ( + re.compile(r".*(CHAR|STRING|TEXT).*", re.IGNORECASE), ), - utils.DbColumnType.TEMPORAL: ( - re.compile(r".*DATE.*", re.IGNORECASE), - re.compile(r".*TIME.*", re.IGNORECASE), + utils.GenericColumnType.TEMPORAL: ( + re.compile(r".*(DATE|TIME).*", re.IGNORECASE), ), } @classmethod def is_db_column_type_match( - cls, db_column_type: Optional[str], target_column_type: utils.DbColumnType + cls, db_column_type: Optional[str], target_column_type: utils.GenericColumnType ) -> bool: """ Check if a column type satisfies a pattern in a collection of regexes found in diff --git a/superset/result_set.py b/superset/result_set.py index d0a325b..367b04f 100644 --- a/superset/result_set.py +++ b/superset/result_set.py @@ -123,7 +123,7 @@ class SupersetResultSet: if pa.types.is_nested(pa_data[i].type): # TODO: revisit nested column serialization once nested types # are added as a natively supported column type in Superset - # (superset.utils.core.DbColumnType). + # (superset.utils.core.GenericColumnType). stringified_arr = stringify_values(array[column]) pa_data[i] = pa.array(stringified_arr.tolist()) @@ -182,7 +182,7 @@ class SupersetResultSet: def is_temporal(self, db_type_str: Optional[str]) -> bool: return self.db_engine_spec.is_db_column_type_match( - db_type_str, utils.DbColumnType.TEMPORAL + db_type_str, utils.GenericColumnType.TEMPORAL ) def data_type(self, col_name: str, pa_dtype: pa.DataType) -> Optional[str]: diff --git a/superset/utils/core.py b/superset/utils/core.py index b4e3ef5..b1ef721 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -37,7 +37,7 @@ from email.mime.image import MIMEImage from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText from email.utils import formatdate -from enum import Enum +from enum import Enum, IntEnum from time import struct_time from timeit import default_timer from types import TracebackType @@ -1424,6 +1424,19 @@ def get_column_names_from_metrics(metrics: List[Metric]) -> List[str]: columns.append(column_name) return columns +def serialize_dtypes(dtypes: List[np.dtype]) -> List[GenericColumnType]: + """Serialize pandas/numpy dtypes to JavaScript types""" + mapping = { + 'object': GenericColumnType.STRING, + 'category': GenericColumnType.STRING, + 'datetime64[ns]': GenericColumnType.TEMPORAL, + 'int64': GenericColumnType.NUMERIC, + 'in32': GenericColumnType.NUMERIC, + 'float64': GenericColumnType.NUMERIC, + 'float32': GenericColumnType.NUMERIC, + 'bool': GenericColumnType.BOOLEAN, + } + return [mapping.get(str(x), GenericColumnType.STRING) for x in dtypes] def indexed( items: List[Any], key: Union[str, Callable[[Any], Any]] @@ -1483,14 +1496,15 @@ class QuerySource(Enum): SQL_LAB = 2 -class DbColumnType(Enum): +class GenericColumnType(IntEnum): """ - Generic database column type + Generic database column type that fits both frontend and backend. """ NUMERIC = 0 STRING = 1 TEMPORAL = 2 + BOOLEAN = 3 class QueryMode(str, LenientEnum): diff --git a/tests/sqla_models_tests.py b/tests/sqla_models_tests.py index f6bb4fc..4657dce 100644 --- a/tests/sqla_models_tests.py +++ b/tests/sqla_models_tests.py @@ -26,7 +26,7 @@ from superset.connectors.sqla.models import SqlaTable, TableColumn from superset.db_engine_specs.druid import DruidEngineSpec from superset.exceptions import QueryObjectValidationError from superset.models.core import Database -from superset.utils.core import DbColumnType, get_example_database, FilterOperator +from superset.utils.core import GenericColumnType, get_example_database, FilterOperator from .base_tests import SupersetTestCase @@ -74,34 +74,34 @@ class TestDatabaseModel(SupersetTestCase): col.is_dttm = True assert col.is_temporal is True - def test_db_column_types(self): - test_cases: Dict[str, DbColumnType] = { + def test_generic_column_types(self): + test_cases: Dict[str, GenericColumnType] = { # string - "CHAR": DbColumnType.STRING, - "VARCHAR": DbColumnType.STRING, - "NVARCHAR": DbColumnType.STRING, - "STRING": DbColumnType.STRING, - "TEXT": DbColumnType.STRING, - "NTEXT": DbColumnType.STRING, + "CHAR": GenericColumnType.STRING, + "VARCHAR": GenericColumnType.STRING, + "NVARCHAR": GenericColumnType.STRING, + "STRING": GenericColumnType.STRING, + "TEXT": GenericColumnType.STRING, + "NTEXT": GenericColumnType.STRING, # numeric - "INT": DbColumnType.NUMERIC, - "BIGINT": DbColumnType.NUMERIC, - "FLOAT": DbColumnType.NUMERIC, - "DECIMAL": DbColumnType.NUMERIC, - "MONEY": DbColumnType.NUMERIC, + "INT": GenericColumnType.NUMERIC, + "BIGINT": GenericColumnType.NUMERIC, + "FLOAT": GenericColumnType.NUMERIC, + "DECIMAL": GenericColumnType.NUMERIC, + "MONEY": GenericColumnType.NUMERIC, # temporal - "DATE": DbColumnType.TEMPORAL, - "DATETIME": DbColumnType.TEMPORAL, - "TIME": DbColumnType.TEMPORAL, - "TIMESTAMP": DbColumnType.TEMPORAL, + "DATE": GenericColumnType.TEMPORAL, + "DATETIME": GenericColumnType.TEMPORAL, + "TIME": GenericColumnType.TEMPORAL, + "TIMESTAMP": GenericColumnType.TEMPORAL, } tbl = SqlaTable(table_name="col_type_test_tbl", database=get_example_database()) for str_type, db_col_type in test_cases.items(): col = TableColumn(column_name="foo", type=str_type, table=tbl) - self.assertEqual(col.is_temporal, db_col_type == DbColumnType.TEMPORAL) - self.assertEqual(col.is_numeric, db_col_type == DbColumnType.NUMERIC) - self.assertEqual(col.is_string, db_col_type == DbColumnType.STRING) + self.assertEqual(col.is_temporal, db_col_type == GenericColumnType.TEMPORAL) + self.assertEqual(col.is_numeric, db_col_type == GenericColumnType.NUMERIC) + self.assertEqual(col.is_string, db_col_type == GenericColumnType.STRING) @patch("superset.jinja_context.g") def test_extra_cache_keys(self, flask_g):
