From f5a841b3207a2af3c7123f2b9bc29cf96328d3b5 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Tue, 16 Jun 2026 11:37:49 -0700 Subject: [PATCH] fix(sql): stop sanitize_clause from rewriting user SQL semantics (#36113) `sanitize_clause` round-tripped the user's clause through SQLGlot's dialect generator and returned the re-rendered SQL. SQLGlot's Postgres dialect (borrowed by engines such as Redshift, CockroachDB, Netezza and SAP HANA) rewrites `ROUND(AVG(x), n)` into `ROUND(CAST(AVG(x) AS DECIMAL), n)`. On engines whose unqualified DECIMAL defaults to scale 0, that injected cast rounds the aggregate to an integer before the explicit ROUND, so `ROUND(AVG(0.949583), 4)` returns `1` instead of `0.9496`. This regressed when validation moved from sqlparse to SQLGlot; the legacy implementation only validated the clause and returned it unchanged. Validate by parsing as before, but return the original clause verbatim. Clauses that contain comments are still re-rendered, since a trailing line comment can comment out surrounding SQL once the clause is embedded into a larger query. Co-Authored-By: Claude Opus 4.8 --- superset/sql/parse.py | 21 +++++++++++++++++++-- tests/unit_tests/sql/parse_tests.py | 3 ++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/superset/sql/parse.py b/superset/sql/parse.py index 93fccc44078..2026b0409b0 100644 --- a/superset/sql/parse.py +++ b/superset/sql/parse.py @@ -1843,16 +1843,33 @@ def process_jinja_sql( def sanitize_clause(clause: str, engine: str) -> str: """ - Make sure the SQL clause is valid. + Validate a SQL clause and return it unchanged. + + The clause is parsed to ensure it is a single, well-formed statement. We + intentionally return the *original* text rather than a re-rendered version: + round-tripping user SQL through SQLGlot's dialect generator can silently + alter semantics. For example, the Postgres dialect (borrowed by several + engines) rewrites ``ROUND(AVG(x), n)`` to ``ROUND(CAST(AVG(x) AS DECIMAL), + n)``, which rounds the value to an integer before the explicit ``ROUND`` on + engines whose unqualified ``DECIMAL`` defaults to scale 0 (see #36113). + + Comments are the one exception: a trailing line comment can comment out + surrounding SQL once the clause is embedded into a larger query (e.g. + wrapped in parentheses), so any clause that contains comments is re-rendered + to normalize them into a safe form. """ try: statement = SQLStatement(clause, engine) + parsed = statement._parsed # pylint: disable=protected-access + if not any(node.comments for node in parsed.walk()): + return clause + return _normalized_generator( SQLGLOT_DIALECTS.get(engine), pretty=False, comments=True, ).generate( - statement._parsed, # pylint: disable=protected-access + parsed, copy=True, ) except SupersetParseError as ex: diff --git a/tests/unit_tests/sql/parse_tests.py b/tests/unit_tests/sql/parse_tests.py index a5be2df2c6c..0f2d7439dda 100644 --- a/tests/unit_tests/sql/parse_tests.py +++ b/tests/unit_tests/sql/parse_tests.py @@ -3123,7 +3123,8 @@ def test_is_valid_cvas(sql: str, engine: str, expected: bool) -> None: "sql, expected, engine", [ ("col = 1", "col = 1", "base"), - ("1=\t\n1", "1 = 1", "base"), + # Comment-free clauses are returned verbatim (no semantic round-trip). + ("1=\t\n1", "1=\t\n1", "base"), ("(col = 1)", "(col = 1)", "base"), # Compact format without newlines ( "(col1 = 1) AND (col2 = 2)",