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


The following commit(s) were added to refs/heads/4.0 by this push:
     new bc7d7b09fc fix: Revert "fix: don't strip SQL comments in Explore 
(#28363)" (#28567)
bc7d7b09fc is described below

commit bc7d7b09fc23c9051afe015b4ace1e1ef95d25eb
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/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 ++--
 7 files changed, 11 insertions(+), 14 deletions(-)

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"
 

Reply via email to