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 e2dfdc9055 fix: don't strip SQL comments in Explore (#28363)
e2dfdc9055 is described below
commit e2dfdc90553e39217792c5f972ecdfae7a380939
Author: Maxime Beauchemin <[email protected]>
AuthorDate: Tue May 7 14:49:46 2024 -0700
fix: don't strip SQL comments in Explore (#28363)
---
superset/connectors/sqla/models.py | 12 ++++++------
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, 20 insertions(+), 17 deletions(-)
diff --git a/superset/connectors/sqla/models.py
b/superset/connectors/sqla/models.py
index 089b9c2f28..0d7b26248f 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -458,9 +458,11 @@ class BaseDatasource(
)
else:
_columns = [
- utils.get_column_name(column_)
- if utils.is_adhoc_column(column_)
- else column_
+ (
+ 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.as_list(form_data.get(column_param)
or [])
]
@@ -757,7 +759,6 @@ class AnnotationDatasource(BaseDatasource):
class TableColumn(AuditMixinNullable, ImportExportMixin, CertificationMixin,
Model):
-
"""ORM object for table columns, each table can have multiple columns"""
__tablename__ = "table_columns"
@@ -971,7 +972,6 @@ class TableColumn(AuditMixinNullable, ImportExportMixin,
CertificationMixin, Mod
class SqlMetric(AuditMixinNullable, ImportExportMixin, CertificationMixin,
Model):
-
"""ORM object for metrics, each table can have multiple metrics"""
__tablename__ = "sql_metrics"
@@ -1492,7 +1492,7 @@ class SqlaTable(
msg=ex.message,
)
) from ex
- sql = sqlparse.format(sql.strip("\t\r\n; "), strip_comments=True)
+ sql = sqlparse.format(sql.strip("\t\r\n; "))
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 5b850913ed..2d41a3c79b 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -916,9 +916,8 @@ 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_statement, cls.top_keywords
+ sql, cls.top_keywords
)
if not limit:
final_limit = query_limit
@@ -927,7 +926,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_statement)
+ cte, sql_remainder = sql_parse.get_cte_remainder_query(sql)
if cte:
str_statement = str(sql_remainder)
cte = cte + "\n"
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 51b771ff61..11c5a91dc5 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; "), strip_comments=True)
+ sql = sqlparse.format(sql.strip("\t\r\n; "))
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 caf9a3cb2b..67bea2b1fb 100644
--- a/superset/sqllab/query_render.py
+++ b/superset/sqllab/query_render.py
@@ -60,7 +60,6 @@ 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 b90416587c..20c620b658 100644
--- a/tests/integration_tests/conftest.py
+++ b/tests/integration_tests/conftest.py
@@ -305,6 +305,7 @@ 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 ef20fc25af..f9a8c407be 100644
--- a/tests/integration_tests/core_tests.py
+++ b/tests/integration_tests/core_tests.py
@@ -539,7 +539,9 @@ class TestCore(SupersetTestCase):
database=get_example_database(),
)
rendered_query = str(table.get_from_clause()[0])
- self.assertEqual(clean_query, rendered_query)
+ assert "comment 1" in rendered_query
+ assert "comment 2" in rendered_query
+ assert "FROM tbl" in 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 91e843fc3f..a36f6da5da 100644
--- a/tests/integration_tests/datasource_tests.py
+++ b/tests/integration_tests/datasource_tests.py
@@ -529,10 +529,12 @@ def test_get_samples(test_client, login_as_admin,
virtual_dataset):
assert "coltypes" in rv2.json["result"]
assert "data" in rv2.json["result"]
- eager_samples = virtual_dataset.database.get_df(
- f"select * from ({virtual_dataset.sql}) as tbl"
- f' limit {app.config["SAMPLES_ROW_LIMIT"]}'
+ sql = (
+ 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 5248aab2eb..b13eccdd90 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 }} {{fake.fn()}}\nLIMIT 10",
+ "SELECT * FROM birth_names WHERE state = '{{ state }}' -- blabblah
{{ extra1 }}\nLIMIT 10",
"3",
- template_params=json.dumps({"state": "CA"}),
+ template_params=json.dumps({"state": "CA", "extra1": "comment"}),
)
assert data["status"] == "success"
