This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 4.0 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 53f98af0fd782c73823721560a7927724f6e9bea Author: Michael S. Molina <[email protected]> AuthorDate: Fri May 17 15:01:23 2024 -0300 fix: Revert "fix: don't strip SQL comments in Explore (#28363)" (#28567) --- superset/connectors/sqla/models.py | 2 +- superset/db_engine_specs/base.py | 5 +++-- superset/models/helpers.py | 2 +- superset/sqllab/query_render.py | 1 + tests/integration_tests/conftest.py | 1 - tests/integration_tests/core_tests.py | 4 +--- tests/integration_tests/datasource_tests.py | 8 +++----- tests/integration_tests/sqllab_tests.py | 4 ++-- 8 files changed, 12 insertions(+), 15 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 0d7b26248f..61fd9fc7f5 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -1492,7 +1492,7 @@ class SqlaTable( msg=ex.message, ) ) from ex - sql = sqlparse.format(sql.strip("\t\r\n; ")) + sql = sqlparse.format(sql.strip("\t\r\n; "), strip_comments=True) if not sql: raise QueryObjectValidationError(_("Virtual dataset query cannot be empty")) if len(sqlparse.split(sql)) > 1: diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 2d41a3c79b..5b850913ed 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -916,8 +916,9 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods cte = None sql_remainder = None sql = sql.strip(" \t\n;") + sql_statement = sqlparse.format(sql, strip_comments=True) query_limit: int | None = sql_parse.extract_top_from_query( - sql, cls.top_keywords + sql_statement, cls.top_keywords ) if not limit: final_limit = query_limit @@ -926,7 +927,7 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods else: final_limit = limit if not cls.allows_cte_in_subquery: - cte, sql_remainder = sql_parse.get_cte_remainder_query(sql) + cte, sql_remainder = sql_parse.get_cte_remainder_query(sql_statement) if cte: str_statement = str(sql_remainder) cte = cte + "\n" diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 11c5a91dc5..51b771ff61 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -1071,7 +1071,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods msg=ex.message, ) ) from ex - sql = sqlparse.format(sql.strip("\t\r\n; ")) + sql = sqlparse.format(sql.strip("\t\r\n; "), strip_comments=True) if not sql: raise QueryObjectValidationError(_("Virtual dataset query cannot be empty")) if len(sqlparse.split(sql)) > 1: diff --git a/superset/sqllab/query_render.py b/superset/sqllab/query_render.py index 67bea2b1fb..caf9a3cb2b 100644 --- a/superset/sqllab/query_render.py +++ b/superset/sqllab/query_render.py @@ -60,6 +60,7 @@ class SqlQueryRenderImpl(SqlQueryRender): parsed_query = ParsedQuery( query_model.sql, + strip_comments=True, engine=query_model.database.db_engine_spec.engine, ) rendered_query = sql_template_processor.process_template( diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py index 20c620b658..b90416587c 100644 --- a/tests/integration_tests/conftest.py +++ b/tests/integration_tests/conftest.py @@ -305,7 +305,6 @@ def virtual_dataset(): "SELECT 3 as col1, 'd' as col2, 1.3, NULL, '2000-01-04 00:00:00', 4 " "UNION ALL " "SELECT 4 as col1, 'e' as col2, 1.4, NULL, '2000-01-05 00:00:00', 5 " - "\n /* CONTAINS A RANDOM COMMENT */ \n" "UNION ALL " "SELECT 5 as col1, 'f' as col2, 1.5, NULL, '2000-01-06 00:00:00', 6 " "UNION ALL " diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index f9a8c407be..ef20fc25af 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -539,9 +539,7 @@ class TestCore(SupersetTestCase): database=get_example_database(), ) rendered_query = str(table.get_from_clause()[0]) - assert "comment 1" in rendered_query - assert "comment 2" in rendered_query - assert "FROM tbl" in rendered_query + self.assertEqual(clean_query, rendered_query) def test_slice_payload_no_datasource(self): form_data = { diff --git a/tests/integration_tests/datasource_tests.py b/tests/integration_tests/datasource_tests.py index a36f6da5da..91e843fc3f 100644 --- a/tests/integration_tests/datasource_tests.py +++ b/tests/integration_tests/datasource_tests.py @@ -529,12 +529,10 @@ def test_get_samples(test_client, login_as_admin, virtual_dataset): assert "coltypes" in rv2.json["result"] assert "data" in rv2.json["result"] - sql = ( - f"select * from ({virtual_dataset.sql}) as tbl " - f'limit {app.config["SAMPLES_ROW_LIMIT"]}' + eager_samples = virtual_dataset.database.get_df( + f"select * from ({virtual_dataset.sql}) as tbl" + f' limit {app.config["SAMPLES_ROW_LIMIT"]}' ) - eager_samples = virtual_dataset.database.get_df(sql) - # the col3 is Decimal eager_samples["col3"] = eager_samples["col3"].apply(float) eager_samples = eager_samples.to_dict(orient="records") diff --git a/tests/integration_tests/sqllab_tests.py b/tests/integration_tests/sqllab_tests.py index b13eccdd90..5248aab2eb 100644 --- a/tests/integration_tests/sqllab_tests.py +++ b/tests/integration_tests/sqllab_tests.py @@ -571,9 +571,9 @@ class TestSqlLab(SupersetTestCase): assert data["status"] == "success" data = self.run_sql( - "SELECT * FROM birth_names WHERE state = '{{ state }}' -- blabblah {{ extra1 }}\nLIMIT 10", + "SELECT * FROM birth_names WHERE state = '{{ state }}' -- blabblah {{ extra1 }} {{fake.fn()}}\nLIMIT 10", "3", - template_params=json.dumps({"state": "CA", "extra1": "comment"}), + template_params=json.dumps({"state": "CA"}), ) assert data["status"] == "success"
