sadpandajoe commented on code in PR #40138:
URL: https://github.com/apache/superset/pull/40138#discussion_r3269940771
##########
tests/unit_tests/sql/parse_tests.py:
##########
@@ -1164,6 +1164,51 @@ def test_has_mutation(engine: str, sql: str, expected:
bool) -> None:
assert SQLScript(sql, engine).has_mutation() == expected
[email protected](
+ "engine",
+ ["oracle", "postgresql", "trino", "presto", "hive", "base"],
+)
+def test_with_clause_containing_union_is_not_mutating(engine: str) -> None:
+ """
+ Regression for #25659: a SELECT with a WITH clause whose CTEs contain
+ UNION (or UNION ALL) must not be classified as mutating, on any dialect.
+
+ The original bug surfaced on Oracle, where saving a virtual dataset built
+ from such a query failed with "Only `SELECT` statements are allowed". The
+ parser was misclassifying the WITH+UNION construct as DML.
+
+ Multiple dialects are exercised because the bug arose from sqlglot's
+ per-dialect AST shape — Oracle's representation of the same query may
+ differ from Postgres/Trino, and a fix that only touches one dialect
+ leaves the others exposed to the same regression.
+ """
+ sql = """
+ WITH set1 AS (SELECT 1 AS n UNION SELECT 2),
+ set2 AS (SELECT * FROM set1)
+ SELECT * FROM set2
+ """
+ assert not SQLScript(sql, engine).has_mutation(), (
+ f"WITH+UNION misclassified as mutating on {engine!r}; "
+ "this would block the query from being saved as a virtual dataset."
+ )
+
+
+def test_with_clause_containing_union_all_is_not_mutating_oracle() -> None:
+ """
+ Companion to test_with_clause_containing_union_is_not_mutating: the
+ original bug report (#25659) used the exact Oracle-flavored shape below
+ (``SYSDATE FROM DUAL`` is the Oracle no-op for "now"). Pinning the
+ verbatim repro guards against a future dialect-specific regression that
+ a generic ``SELECT 1`` test might miss.
+ """
Review Comment:
"""
Companion to test_with_clause_containing_union_is_not_mutating: exercises
UNION ALL (a sibling of UNION whose per-dialect sqlglot AST shape can
diverge) on the Oracle-flavored ``SYSDATE FROM DUAL`` form from #25659.
UNION is covered by the parametrized test above; this guards against a
UNION-ALL-specific or Oracle-shape-specific regression that a generic
``SELECT 1 UNION SELECT 2`` test might miss.
"""
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
