From d89f4ce6785933b4487f3ad064a078b93f8eeda9 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 17 Mar 2022 16:37:20 +0800 Subject: [PATCH 1/5] fix: adhoc column with legacy chart --- superset/connectors/base/models.py | 9 ++++++--- tests/integration_tests/model_tests.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py index 967235f328c2e..b6b294955d4e9 100644 --- a/superset/connectors/base/models.py +++ b/superset/connectors/base/models.py @@ -339,11 +339,14 @@ def data_for_slices( # pylint: disable=too-many-locals or [] ) else: - column_names.update( - column + _columns = [ + utils.get_column_name(column) + if utils.is_adhoc_column(column) + else column for column_param in COLUMN_FORM_DATA_PARAMS for column in utils.get_iterable(form_data.get(column_param) or []) - ) + ] + column_names.update(_columns) filtered_metrics = [ metric diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py index 6371a06123ae8..d2bc7a674712e 100644 --- a/tests/integration_tests/model_tests.py +++ b/tests/integration_tests/model_tests.py @@ -15,10 +15,13 @@ # specific language governing permissions and limitations # under the License. # isort:skip_file +import json import textwrap import unittest +from pprint import pprint from unittest import mock +from superset.connectors.sqla.models import SqlaTable from superset.exceptions import SupersetException from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, @@ -578,6 +581,23 @@ def test_data_for_slices_with_query_context(self): "state", } + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_data_for_slices_with_adhoc_column(self): + tbl = self.get_table(name="birth_names") + slc = ( + metadata_db.session.query(Slice) + .filter_by( + datasource_id=tbl.id, datasource_type=tbl.type, slice_name="Boys", + ) + .first() + ) + form_data = json.loads(slc.params) + form_data["groupby"].append({"label": "adhoc_column", "sqlExpression": "name"}) + slc.params = json.dumps(form_data) + ds: SqlaTable = slc.datasource + datasource_info = ds.data_for_slices([slc]) + assert "database" in datasource_info + def test_literal_dttm_type_factory(): orig_type = DateTime() From f6981fc3b641a6f38b1da8a859d16594d9220b18 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 17 Mar 2022 16:40:52 +0800 Subject: [PATCH 2/5] comment --- tests/integration_tests/model_tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py index d2bc7a674712e..4274f1c0a7d3a 100644 --- a/tests/integration_tests/model_tests.py +++ b/tests/integration_tests/model_tests.py @@ -583,6 +583,8 @@ def test_data_for_slices_with_query_context(self): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_data_for_slices_with_adhoc_column(self): + # should perform sqla.model.BaseDatasource.data_for_slices() with adhoc + # column and legacy chart tbl = self.get_table(name="birth_names") slc = ( metadata_db.session.query(Slice) From ee1dd3a4d48e8b205e55131d424b6dc5fc8cf025 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Fri, 18 Mar 2022 12:26:41 +0800 Subject: [PATCH 3/5] refine ut --- tests/integration_tests/model_tests.py | 32 +++++++++++++++++--------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py index 4274f1c0a7d3a..71187d21d9028 100644 --- a/tests/integration_tests/model_tests.py +++ b/tests/integration_tests/model_tests.py @@ -586,18 +586,28 @@ def test_data_for_slices_with_adhoc_column(self): # should perform sqla.model.BaseDatasource.data_for_slices() with adhoc # column and legacy chart tbl = self.get_table(name="birth_names") - slc = ( - metadata_db.session.query(Slice) - .filter_by( - datasource_id=tbl.id, datasource_type=tbl.type, slice_name="Boys", - ) - .first() + dashboard = self.get_dash_by_slug("births") + slc = Slice( + slice_name="slice with adhoc column", + datasource_type="table", + viz_type="table", + params=json.dumps( + { + "adhoc_filters": [], + "granularity_sqla": "ds", + "groupby": [ + "name", + {"label": "adhoc_column", "sqlExpression": "name"}, + ], + "metrics": ["sum__num"], + "time_range": "No filter", + "viz_type": "table", + } + ), + datasource_id=tbl.id, ) - form_data = json.loads(slc.params) - form_data["groupby"].append({"label": "adhoc_column", "sqlExpression": "name"}) - slc.params = json.dumps(form_data) - ds: SqlaTable = slc.datasource - datasource_info = ds.data_for_slices([slc]) + dashboard.slices.append(slc) + datasource_info = slc.datasource.data_for_slices([slc]) assert "database" in datasource_info From 81acfec1a3ce84b518a5c20dce5520c4ad62020c Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Fri, 18 Mar 2022 12:28:34 +0800 Subject: [PATCH 4/5] remove pprint --- tests/integration_tests/model_tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py index 71187d21d9028..35e1a1e76bd5c 100644 --- a/tests/integration_tests/model_tests.py +++ b/tests/integration_tests/model_tests.py @@ -18,7 +18,6 @@ import json import textwrap import unittest -from pprint import pprint from unittest import mock from superset.connectors.sqla.models import SqlaTable From c78fc0446bb0c903d1fc9da06c48c722c3bc89c9 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Fri, 18 Mar 2022 13:03:31 +0800 Subject: [PATCH 5/5] records clean up --- tests/integration_tests/model_tests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py index 35e1a1e76bd5c..5ffa65e583ead 100644 --- a/tests/integration_tests/model_tests.py +++ b/tests/integration_tests/model_tests.py @@ -609,6 +609,9 @@ def test_data_for_slices_with_adhoc_column(self): datasource_info = slc.datasource.data_for_slices([slc]) assert "database" in datasource_info + # clean up and auto commit + metadata_db.session.delete(slc) + def test_literal_dttm_type_factory(): orig_type = DateTime()