You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by yj...@apache.org on 2020/11/27 04:53:36 UTC
[incubator-superset] 01/01: refactor: migrate table chart to new API
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 <je...@airbnb.com>
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):