From 0228ff4f33dc781da1dcb84b28858cc538fe33bf Mon Sep 17 00:00:00 2001
From: Evan <evan@preset.io>
Date: Tue, 23 Jun 2026 18:02:36 -0700
Subject: [PATCH] fix(sql): normalize comment clauses with base dialect to keep
 #36113 fix

The comment-handling branch of sanitize_clause re-rendered through the
engine dialect, which re-applied the Postgres ROUND/CAST rewrite for any
clause containing a comment, reintroducing #36113. Re-render with the
base dialect instead so comments are normalized without engine-specific
semantic rewrites, and add a parametrized regression test for the
comment path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 superset/sql/parse.py               | 12 ++++++++----
 tests/unit_tests/sql/parse_tests.py | 23 +++++++++++++++++++++++
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/superset/sql/parse.py b/superset/sql/parse.py
index 1d0bfe8b4e6..20824318e2f 100644
--- a/superset/sql/parse.py
+++ b/superset/sql/parse.py
@@ -1856,9 +1856,13 @@ def sanitize_clause(clause: str, engine: str) -> str:
     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. A trailing statement terminator is
-    likewise stripped, since callers embed the clause inside a larger fragment
-    (``WHERE (...)``) where a stray ``;`` would produce invalid SQL.
+    to normalize them into a safe form. That re-rendering uses the *base* dialect
+    rather than the engine dialect, so it normalizes comments without re-applying
+    the engine-specific rewrites (e.g. the Postgres ``ROUND``/``CAST`` rewrite
+    from #36113) that we deliberately avoid above. A trailing statement
+    terminator is likewise stripped, since callers embed the clause inside a
+    larger fragment (``WHERE (...)``) where a stray ``;`` would produce invalid
+    SQL.
     """
     try:
         statement = SQLStatement(clause, engine)
@@ -1867,7 +1871,7 @@ def sanitize_clause(clause: str, engine: str) -> str:
             return clause.rstrip().rstrip(";").rstrip()
 
         return _normalized_generator(
-            SQLGLOT_DIALECTS.get(engine),
+            None,
             pretty=False,
             comments=True,
         ).generate(
diff --git a/tests/unit_tests/sql/parse_tests.py b/tests/unit_tests/sql/parse_tests.py
index 8342a5ef264..351989e7481 100644
--- a/tests/unit_tests/sql/parse_tests.py
+++ b/tests/unit_tests/sql/parse_tests.py
@@ -3238,6 +3238,29 @@ def test_sanitize_clause_preserves_aggregation_semantics(engine: str) -> None:
     assert sanitized == clause
 
 
+@pytest.mark.parametrize(
+    "engine",
+    ["postgresql", "redshift", "cockroachdb", "netezza", "hana", "base", "mysql"],
+)
+def test_sanitize_clause_preserves_aggregation_semantics_with_comment(
+    engine: str,
+) -> None:
+    """
+    Regression test for https://github.com/apache/superset/issues/36113.
+
+    A clause that contains a comment takes the re-rendering branch of
+    ``sanitize_clause``. That branch must normalize comments using the *base*
+    dialect rather than the engine dialect, so it must not re-apply the Postgres
+    ``ROUND(AVG(x), n)`` -> ``ROUND(CAST(AVG(x) AS DECIMAL), n)`` rewrite that
+    truncates results on engines where ``DECIMAL`` defaults to scale 0.
+    """
+    clause = "ROUND(AVG(col), 4) /* precise_count_distinct=true */"
+    sanitized = sanitize_clause(clause, engine)
+    assert "CAST" not in sanitized.upper(), (
+        f"sanitize_clause injected a cast for engine {engine!r}: {sanitized!r}"
+    )
+
+
 @pytest.mark.parametrize(
     "engine",
     [