Skip to content

Commit

Permalink
omit strip_comments from ParsedQuery
Browse files Browse the repository at this point in the history
  • Loading branch information
dvchristianbors committed Feb 22, 2024
1 parent b991917 commit 41581e9
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 23 deletions.
4 changes: 2 additions & 2 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1564,7 +1564,7 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
# def DATASET_HEALTH_CHECK(datasource: SqlaTable) -> Optional[str]:
# if (
# datasource.sql and
# len(sql_parse.ParsedQuery(datasource.sql, strip_comments=True).tables) == 1
# len(sql_parse.ParsedQuery(datasource.sql).tables) == 1
# ):
# return (
# "This virtual dataset queries only one table and therefore could be "
Expand Down Expand Up @@ -1702,7 +1702,7 @@ class ExtraDynamicQueryFilters(TypedDict, total=False):
try:
# pylint: disable=import-error,wildcard-import,unused-wildcard-import
import superset_config
from superset_config import * # type: ignore
from superset_config import *

print(f"Loaded your LOCAL configuration at [{superset_config.__file__}]")
except Exception:
Expand Down
2 changes: 1 addition & 1 deletion superset/models/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,6 @@ def get_query_str_extended(
sqlaq = self.get_sqla_query(**query_obj)
sql = self.database.compile_sqla_query(sqlaq.sqla_query)
sql = self._apply_cte(sql, sqlaq.cte)
sql = sqlparse.format(sql, reindent=True)
if mutate:
sql = self.mutate_query_from_config(sql)
return QueryStringExtended(
Expand Down Expand Up @@ -1071,6 +1070,7 @@ def get_rendered_sql(
msg=ex.message,
)
) from ex
logger.info(f"{__name__} - get_rendered_sql: Stripping comments from SQL")
sql = sqlparse.format(sql.strip("\t\r\n; "), strip_comments=True)
if not sql:
raise QueryObjectValidationError(_("Virtual dataset query cannot be empty"))
Expand Down
1 change: 0 additions & 1 deletion superset/sql_lab.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,6 @@ def execute_sql_statements(
# Breaking down into multiple statements
parsed_query = ParsedQuery(
rendered_query,
strip_comments=True,
engine=db_engine_spec.engine,
)
if not db_engine_spec.run_multiple_statements_as_one:
Expand Down
5 changes: 1 addition & 4 deletions superset/sql_parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,8 @@ class ParsedQuery:
def __init__(
self,
sql_statement: str,
strip_comments: bool = False,
engine: Optional[str] = None,
):
if strip_comments:
sql_statement = sqlparse.format(sql_statement, strip_comments=True)

self.sql: str = sql_statement
self._dialect = SQLGLOT_DIALECTS.get(engine) if engine else None
self._tables: set[Table] = set()
Expand Down Expand Up @@ -604,6 +600,7 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:


def sanitize_clause(clause: str) -> str:
logger.info(f"{__name__}: sanitize")
# clause = sqlparse.format(clause, strip_comments=True)
statements = sqlparse.parse(clause)
if len(statements) != 1:
Expand Down
1 change: 0 additions & 1 deletion superset/sqllab/query_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ def render(self, execution_context: SqlJsonExecutionContext) -> str:

parsed_query = ParsedQuery(
query_model.sql,
strip_comments=True,
engine=query_model.database.db_engine_spec.engine,
)
rendered_query = sql_template_processor.process_template(
Expand Down
16 changes: 2 additions & 14 deletions tests/unit_tests/sql_parse_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,9 +863,7 @@ def test_is_valid_ctas() -> None:
A valid CTAS has a ``SELECT`` as its last statement.
"""
assert (
ParsedQuery("SELECT * FROM table", strip_comments=True).is_valid_ctas() is True
)
assert ParsedQuery("SELECT * FROM table").is_valid_ctas() is True

assert (
ParsedQuery(
Expand All @@ -874,7 +872,6 @@ def test_is_valid_ctas() -> None:
SELECT * FROM table
-- comment 2
""",
strip_comments=True,
).is_valid_ctas()
is True
)
Expand All @@ -887,7 +884,6 @@ def test_is_valid_ctas() -> None:
SELECT @value as foo;
-- comment 2
""",
strip_comments=True,
).is_valid_ctas()
is True
)
Expand All @@ -899,7 +895,6 @@ def test_is_valid_ctas() -> None:
EXPLAIN SELECT * FROM table
-- comment 2
""",
strip_comments=True,
).is_valid_ctas()
is False
)
Expand All @@ -910,7 +905,6 @@ def test_is_valid_ctas() -> None:
SELECT * FROM table;
INSERT INTO TABLE (foo) VALUES (42);
""",
strip_comments=True,
).is_valid_ctas()
is False
)
Expand All @@ -922,9 +916,7 @@ def test_is_valid_cvas() -> None:
A valid CVAS has a single ``SELECT`` statement.
"""
assert (
ParsedQuery("SELECT * FROM table", strip_comments=True).is_valid_cvas() is True
)
assert ParsedQuery("SELECT * FROM table").is_valid_cvas() is True

assert (
ParsedQuery(
Expand All @@ -933,7 +925,6 @@ def test_is_valid_cvas() -> None:
SELECT * FROM table
-- comment 2
""",
strip_comments=True,
).is_valid_cvas()
is True
)
Expand All @@ -946,7 +937,6 @@ def test_is_valid_cvas() -> None:
SELECT @value as foo;
-- comment 2
""",
strip_comments=True,
).is_valid_cvas()
is False
)
Expand All @@ -958,7 +948,6 @@ def test_is_valid_cvas() -> None:
EXPLAIN SELECT * FROM table
-- comment 2
""",
strip_comments=True,
).is_valid_cvas()
is False
)
Expand All @@ -969,7 +958,6 @@ def test_is_valid_cvas() -> None:
SELECT * FROM table;
INSERT INTO TABLE (foo) VALUES (42);
""",
strip_comments=True,
).is_valid_cvas()
is False
)
Expand Down

0 comments on commit 41581e9

Please sign in to comment.