You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2024/10/04 15:09:43 UTC
(superset) branch master updated: fix: don't reformat generated queries (#30350)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 0b34197815 fix: don't reformat generated queries (#30350)
0b34197815 is described below
commit 0b34197815ee0432e7dac1b38f9075e09ea1e096
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Fri Oct 4 11:09:37 2024 -0400
fix: don't reformat generated queries (#30350)
---
.../cypress/e2e/explore/AdhocMetrics.test.ts | 2 +-
.../e2e/explore/visualizations/table.test.ts | 4 +--
superset/models/helpers.py | 7 +---
tests/integration_tests/charts/data/api_tests.py | 8 ++---
tests/integration_tests/datasource_tests.py | 2 +-
.../db_engine_specs/base_engine_spec_tests.py | 2 +-
.../db_engine_specs/bigquery_tests.py | 2 +-
tests/integration_tests/query_context_tests.py | 9 +++--
.../security/row_level_security_tests.py | 2 +-
tests/integration_tests/sqla_models_tests.py | 2 +-
tests/unit_tests/jinja_context_test.py | 39 ++++++----------------
11 files changed, 29 insertions(+), 50 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/AdhocMetrics.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/AdhocMetrics.test.ts
index fe861c3be4..f843f258f9 100644
--- a/superset-frontend/cypress-base/cypress/e2e/explore/AdhocMetrics.test.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/explore/AdhocMetrics.test.ts
@@ -25,7 +25,7 @@ describe('AdhocMetrics', () => {
});
it('Clear metric and set simple adhoc metric', () => {
- const metric = 'SUM(num_girls)';
+ const metric = 'sum(num_girls)';
const metricName = 'Sum Girls';
cy.get('[data-test=metrics]')
.find('[data-test="remove-control-button"]')
diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/table.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/table.test.ts
index 6d93cd47f4..603cc7f2c8 100644
--- a/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/table.test.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/table.test.ts
@@ -100,7 +100,7 @@ describe('Visualization > Table', () => {
});
cy.verifySliceSuccess({
waitAlias: '@chartData',
- querySubstring: /group by\n.*name/i,
+ querySubstring: /GROUP BY.*name/i,
chartSelector: 'table',
});
});
@@ -247,7 +247,7 @@ describe('Visualization > Table', () => {
cy.visitChartByParams(formData);
cy.verifySliceSuccess({
waitAlias: '@chartData',
- querySubstring: /group by\n.*state/i,
+ querySubstring: /GROUP BY.*state/i,
chartSelector: 'table',
});
cy.get('td').contains(/\d*%/);
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 80e66f5027..850a6e259f 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -63,12 +63,11 @@ from superset.exceptions import (
ColumnNotFoundException,
QueryClauseValidationException,
QueryObjectValidationError,
- SupersetParseError,
SupersetSecurityException,
)
from superset.extensions import feature_flag_manager
from superset.jinja_context import BaseTemplateProcessor
-from superset.sql.parse import SQLScript, SQLStatement
+from superset.sql.parse import SQLScript
from superset.sql_parse import (
has_table_query,
insert_rls_in_predicate,
@@ -870,10 +869,6 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
sqlaq = self.get_sqla_query(**query_obj)
sql = self.database.compile_sqla_query(sqlaq.sqla_query)
sql = self._apply_cte(sql, sqlaq.cte)
- try:
- sql = SQLStatement(sql, engine=self.db_engine_spec.engine).format()
- except SupersetParseError:
- logger.warning("Unable to parse SQL to format it, passing it as-is")
if mutate:
sql = self.database.mutate_sql_based_on_config(sql)
diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py
index abfc4dd83a..9aeac84cc5 100644
--- a/tests/integration_tests/charts/data/api_tests.py
+++ b/tests/integration_tests/charts/data/api_tests.py
@@ -716,7 +716,7 @@ class TestPostChartDataApi(BaseTestChartDataApi):
rv = self.post_assert_metric(CHART_DATA_URI, self.query_context_payload, "data")
result = rv.json["result"][0]["query"]
if get_example_database().backend != "presto":
- assert "(\n 'boy' = 'boy'\n )" in result
+ assert "('boy' = 'boy')" in result
@unittest.skip("Extremely flaky test on MySQL")
@with_feature_flags(GLOBAL_ASYNC_QUERIES=True)
@@ -1347,13 +1347,13 @@ def test_time_filter_with_grain(test_client, login_as_admin, physical_query_cont
backend = get_example_database().backend
if backend == "sqlite":
assert (
- "DATETIME(col5, 'start of day', -STRFTIME('%w', col5) || ' days') >="
+ "DATETIME(col5, 'start of day', -strftime('%w', col5) || ' days') >="
in query
)
elif backend == "mysql":
- assert "DATE(DATE_SUB(col5, INTERVAL (DAYOFWEEK(col5) - 1) DAY)) >=" in query
+ assert "DATE(DATE_SUB(col5, INTERVAL DAYOFWEEK(col5) - 1 DAY)) >=" in query
elif backend == "postgresql":
- assert "DATE_TRUNC('WEEK', col5) >=" in query
+ assert "DATE_TRUNC('week', col5) >=" in query
elif backend == "presto":
assert "date_trunc('week', CAST(col5 AS TIMESTAMP)) >=" in query
diff --git a/tests/integration_tests/datasource_tests.py b/tests/integration_tests/datasource_tests.py
index a2f21e7fc7..a6c316fd1e 100644
--- a/tests/integration_tests/datasource_tests.py
+++ b/tests/integration_tests/datasource_tests.py
@@ -692,7 +692,7 @@ def test_get_samples_with_multiple_filters(
assert "2000-01-02" in rv.json["result"]["query"]
assert "2000-01-04" in rv.json["result"]["query"]
assert "col3 = 1.2" in rv.json["result"]["query"]
- assert "col4 IS NULL" in rv.json["result"]["query"]
+ assert "col4 is null" in rv.json["result"]["query"]
assert "col2 = 'c'" in rv.json["result"]["query"]
diff --git a/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py b/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py
index c8db1f912a..215a01f585 100644
--- a/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py
+++ b/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py
@@ -308,7 +308,7 @@ class TestDbEngineSpecs(TestDbEngineSpec):
}
sql = table.get_query_str(query_obj)
assert (
- "ORDER BY\n CASE WHEN gender = 'boy' THEN 'male' ELSE 'female' END ASC"
+ "ORDER BY \n case\n when gender='boy' then 'male'\n else 'female'\n end\n ASC"
in sql
)
diff --git a/tests/integration_tests/db_engine_specs/bigquery_tests.py b/tests/integration_tests/db_engine_specs/bigquery_tests.py
index 889c0295cf..3518d2cf8a 100644
--- a/tests/integration_tests/db_engine_specs/bigquery_tests.py
+++ b/tests/integration_tests/db_engine_specs/bigquery_tests.py
@@ -380,4 +380,4 @@ class TestBigQueryDbEngineSpec(TestDbEngineSpec):
"orderby": [["gender_cc", True]],
}
sql = table.get_query_str(query_obj)
- assert "ORDER BY\n `gender_cc` ASC" in sql
+ assert "ORDER BY `gender_cc` ASC" in sql
diff --git a/tests/integration_tests/query_context_tests.py b/tests/integration_tests/query_context_tests.py
index 2fcd6d2048..9df1b3a872 100644
--- a/tests/integration_tests/query_context_tests.py
+++ b/tests/integration_tests/query_context_tests.py
@@ -367,7 +367,7 @@ class TestQueryContext(SupersetTestCase):
sql_text = get_sql_text(payload)
assert "SELECT" in sql_text
- assert re.search(r'NOT [`"\[]?num[`"\]]? IS NULL', sql_text)
+ assert re.search(r'[`"\[]?num[`"\]]? IS NOT NULL', sql_text)
assert re.search(
r"""NOT \([\s\n]*[`"\[]?name[`"\]]? IS NULL[\s\n]* """
r"""OR [`"\[]?name[`"\]]? IN \('"abc"'\)[\s\n]*\)""",
@@ -1161,16 +1161,19 @@ LIMIT 10000
OFFSET 0
"""
assert (
- re.search(r"WHERE\n col6 >= .*2002-01-01", sqls[0])
+ re.search(r"WHERE col6 >= .*2002-01-01", sqls[0])
and re.search(r"AND col6 < .*2003-01-01", sqls[0])
) is not None
assert (
- re.search(r"WHERE\n col6 >= .*2001-10-01", sqls[1])
+ re.search(r"WHERE col6 >= .*2001-10-01", sqls[1])
and re.search(r"AND col6 < .*2002-10-01", sqls[1])
) is not None
def test_virtual_dataset_with_comments(app_context, virtual_dataset_with_comments):
+ if backend() == "mysql":
+ return
+
qc = QueryContextFactory().create(
datasource={
"type": virtual_dataset_with_comments.type,
diff --git a/tests/integration_tests/security/row_level_security_tests.py b/tests/integration_tests/security/row_level_security_tests.py
index ea744bd9f8..ffd38bd533 100644
--- a/tests/integration_tests/security/row_level_security_tests.py
+++ b/tests/integration_tests/security/row_level_security_tests.py
@@ -268,7 +268,7 @@ class TestRowLevelSecurity(SupersetTestCase):
# establish that the filters are grouped together correctly with
# ANDs, ORs and parens in the correct place
assert (
- "WHERE\n (\n (\n name LIKE 'A%' OR name LIKE 'B%'\n ) OR (\n name LIKE 'Q%'\n )\n )\n AND (\n gender = 'boy'\n )"
+ "WHERE ((name like 'A%' or name like 'B%') OR (name like 'Q%')) AND (gender = 'boy');"
in sql
)
diff --git a/tests/integration_tests/sqla_models_tests.py b/tests/integration_tests/sqla_models_tests.py
index 4398d75c12..ca08842ebe 100644
--- a/tests/integration_tests/sqla_models_tests.py
+++ b/tests/integration_tests/sqla_models_tests.py
@@ -803,7 +803,7 @@ def test_none_operand_in_filter(login_as_admin, physical_dataset):
{
"operator": FilterOperator.NOT_EQUALS.value,
"count": 0,
- "sql_should_contain": "NOT COL4 IS NULL",
+ "sql_should_contain": "COL4 IS NOT NULL",
},
]
for expected in expected_results:
diff --git a/tests/unit_tests/jinja_context_test.py b/tests/unit_tests/jinja_context_test.py
index 5791310018..ced40c8119 100644
--- a/tests/unit_tests/jinja_context_test.py
+++ b/tests/unit_tests/jinja_context_test.py
@@ -470,48 +470,29 @@ def test_dataset_macro(mocker: MockerFixture) -> None:
return_value=[],
)
+ space = " "
+
assert (
dataset_macro(1)
- == """(
-SELECT
- ds AS ds,
- num_boys AS num_boys,
- revenue AS revenue,
- expenses AS expenses,
- revenue - expenses AS profit
+ == f"""(
+SELECT ds AS ds, num_boys AS num_boys, revenue AS revenue, expenses AS expenses, revenue-expenses AS profit{space}
FROM my_schema.old_dataset
) AS dataset_1"""
)
assert (
dataset_macro(1, include_metrics=True)
- == """(
-SELECT
- ds AS ds,
- num_boys AS num_boys,
- revenue AS revenue,
- expenses AS expenses,
- revenue - expenses AS profit,
- COUNT(*) AS cnt
-FROM my_schema.old_dataset
-GROUP BY
- ds,
- num_boys,
- revenue,
- expenses,
- revenue - expenses
+ == f"""(
+SELECT ds AS ds, num_boys AS num_boys, revenue AS revenue, expenses AS expenses, revenue-expenses AS profit, COUNT(*) AS cnt{space}
+FROM my_schema.old_dataset GROUP BY ds, num_boys, revenue, expenses, revenue-expenses
) AS dataset_1"""
)
assert (
dataset_macro(1, include_metrics=True, columns=["ds"])
- == """(
-SELECT
- ds AS ds,
- COUNT(*) AS cnt
-FROM my_schema.old_dataset
-GROUP BY
- ds
+ == f"""(
+SELECT ds AS ds, COUNT(*) AS cnt{space}
+FROM my_schema.old_dataset GROUP BY ds
) AS dataset_1"""
)