From b4f3fae288011beea3e9da3af6704317ae9287fb Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Tue, 11 Aug 2026 23:33:15 -0700 Subject: [PATCH] fix(sql): guard FORCE_LIMIT against SHOW statements (#36939) (#42588) Co-authored-by: Claude Code --- superset/sql/parse.py | 9 ++++++ tests/unit_tests/sql/parse_tests.py | 46 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/superset/sql/parse.py b/superset/sql/parse.py index c194c4e63ef..1036772071e 100644 --- a/superset/sql/parse.py +++ b/superset/sql/parse.py @@ -1407,6 +1407,15 @@ class SQLStatement(BaseSQLStatement[exp.Expression]): Modify the `LIMIT` or `TOP` value of the SQL statement inplace. """ if method == LimitMethod.FORCE_LIMIT: + # `SHOW` statements (`SHOW TABLES`, `SHOW DATABASES`, `SHOW CREATE + # TABLE`, etc.) have no meaningful `LIMIT` slot to force. On + # MySQL/StarRocks, writing one renders a malformed statement with + # two `LIMIT` keywords that the engine rejects outright; on dialects + # like Snowflake it would render a valid `SHOW ... LIMIT`, but SHOW + # returns bounded metadata, so we skip it uniformly rather than + # special-case per dialect. Leave them untouched. + if isinstance(self._parsed, exp.Show): + return self._parsed.args["limit"] = exp.Limit( expression=exp.Literal(this=str(limit), is_string=False) ) diff --git a/tests/unit_tests/sql/parse_tests.py b/tests/unit_tests/sql/parse_tests.py index 0e6b13f86ba..e8043d750af 100644 --- a/tests/unit_tests/sql/parse_tests.py +++ b/tests/unit_tests/sql/parse_tests.py @@ -2387,6 +2387,52 @@ def test_set_limit_value( assert statement.format() == expected +@pytest.mark.parametrize( + "engine", + [ + # Engines whose sqlglot dialect parses `SHOW` into a real `exp.Show` + # node (as opposed to falling back to an opaque `exp.Command`, which + # doesn't expose a `limit` arg and so was never affected by this bug). + "starrocks", + "mysql", + "snowflake", + ], +) +@pytest.mark.parametrize( + "sql", + [ + "SHOW TABLES", + "SHOW DATABASES", + "SHOW CREATE TABLE test.will_test1", + ], +) +def test_set_limit_value_leaves_show_statements_unchanged( + sql: str, engine: 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. + + Covers multiple engines, not just StarRocks: the fix guards on the AST + node type (``exp.Show``), not the dialect, so any engine whose sqlglot + dialect parses ``SHOW`` into a real ``Show`` node (e.g. MySQL, Snowflake) + is equally exposed and must be equally protected. + """ + statement = SQLStatement(sql, engine) + original = statement.format() + statement.set_limit_value(1000, LimitMethod.FORCE_LIMIT) + assert statement.format() == original + assert "LIMIT" not in statement.format() + + @pytest.mark.parametrize( "kql, limit, expected", [