rusackas commented on code in PR #42588:
URL: https://github.com/apache/superset/pull/42588#discussion_r3679240050
##########
tests/unit_tests/sql/parse_tests.py:
##########
@@ -2243,6 +2243,34 @@ def test_set_limit_value(
assert statement.format() == expected
[email protected](
+ "sql",
+ [
+ "SHOW TABLES",
+ "SHOW DATABASES",
+ "SHOW CREATE TABLE test.will_test1",
+ ],
+)
+def test_set_limit_value_leaves_show_statements_unchanged(sql: str) -> None:
+ """
+ Regression for #36939: FORCE_LIMIT must not touch ``SHOW`` statements.
+
+ ``SHOW`` statements have no `LIMIT` clause in sqlglot's expression tree,
+ so forcing one via ``args["limit"]`` doesn't reject cleanly, it produces
+ a malformed statement with two ``LIMIT`` keywords (one from a stray
+ rendering of the bare ``Limit`` expression, one from the forced value).
+ StarRocks (and presumably other engines) reject that outright: "Getting
+ syntax error ... Unexpected input 'LIMIT'". The statement should be
+ left untouched instead, matching how ``SELECT`` statements without a
+ scannable row source aren't force-limited either.
+ """
+ statement = SQLStatement(sql, "starrocks")
Review Comment:
Fair point — added mysql and snowflake to the parametrization alongside
starrocks. The fix guards on `isinstance(self._parsed, exp.Show)`, not the
dialect, so any engine whose sqlglot dialect actually parses `SHOW` into a real
`Show` node is equally exposed; those three cover that.
--
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]
