diff --git a/superset/sql/parse.py b/superset/sql/parse.py index 163bf92b9e2..ff8ee6b1db7 100644 --- a/superset/sql/parse.py +++ b/superset/sql/parse.py @@ -894,7 +894,7 @@ class SQLStatement(BaseSQLStatement[exp.Expression]): sources = cls._split_source(script, engine, len(asts)) return [ cls(ast=ast, engine=engine, source=source) - for ast, source in zip(asts, sources, strict=False) + for ast, source in zip(asts, sources, strict=True) ] @classmethod @@ -927,13 +927,15 @@ class SQLStatement(BaseSQLStatement[exp.Expression]): if tok.token_type == sqlglot.tokens.TokenType.L_PAREN: depth += 1 elif tok.token_type == sqlglot.tokens.TokenType.R_PAREN: - depth -= 1 + # Clamp at 0 so malformed SQL with unbalanced ')' can't drive + # depth negative and misclassify later semicolons as nested. + depth = max(0, depth - 1) elif tok.token_type == sqlglot.tokens.TokenType.SEMICOLON and depth == 0: boundaries.append(tok.start) starts = [0, *(b + 1 for b in boundaries)] ends = [*boundaries, len(script)] - sources = [script[s:e].strip() for s, e in zip(starts, ends, strict=False)] + sources = [script[s:e].strip() for s, e in zip(starts, ends, strict=True)] sources = [s for s in sources if s] if len(sources) != expected_count: return none_result diff --git a/superset/sql/rls_splice.py b/superset/sql/rls_splice.py index b068305cb26..d35d63e2b29 100644 --- a/superset/sql/rls_splice.py +++ b/superset/sql/rls_splice.py @@ -41,6 +41,13 @@ Known limitations: parsing, not generation. - Predicate strings are spliced in as raw SQL. They must come from a trusted source (the RLS config), not user input. + - Predicate **column qualification** (prefixing bare columns with the table + alias) currently round-trips the predicate through the sqlglot generator + via ``_qualify_predicate``. Predicates that contain dialect-specific + functions can therefore still be transpiled by the generator at that step, + even though the surrounding query is preserved byte-for-byte. The + surrounding-query fidelity guarantee does not extend to the predicate + string itself. """ from __future__ import annotations @@ -220,7 +227,6 @@ def _splices_for_scope( join_splice = _find_join_splice(sql, tokens, table_end, pred_sql) if join_splice: join_splices.extend(join_splice) - continue if not from_predicates: return join_splices @@ -237,9 +243,10 @@ def _splices_for_scope( def _table_end(source: exp.Table) -> int | None: ident = source.find(exp.Identifier) - if ident and getattr(ident, "_meta", None): - return ident._meta["end"] - return None + meta = getattr(ident, "_meta", None) if ident else None + if meta is None: + return None + return meta.get("end") def _classify_source_predicate( @@ -284,6 +291,11 @@ def _qualify_predicate( """ Qualify predicate columns with the table alias/name, mirroring ``RLSAsPredicateTransformer``. + + Note: this re-renders the predicate via the sqlglot generator, so the + splice-mode fidelity guarantee does not extend to the predicate text + itself. Predicates containing dialect-specific functions may be transpiled + here even though the surrounding query is preserved byte-for-byte. """ parsed = sqlglot.parse_one(predicate, dialect=dialect) table = table_node.alias_or_name diff --git a/tests/unit_tests/sql/parse_tests.py b/tests/unit_tests/sql/parse_tests.py index 553f6cef3cb..b187ece8250 100644 --- a/tests/unit_tests/sql/parse_tests.py +++ b/tests/unit_tests/sql/parse_tests.py @@ -3298,6 +3298,31 @@ def test_rls_predicate_splice_preserves_dialect_function() -> None: ) +def test_rls_predicate_splice_combines_multiple_predicates() -> None: + """ + Splice mode should AND together multiple predicates configured for the same + table into a single injected condition. + """ + sql = "SELECT * FROM some_table WHERE status = 'open'" + statement = SQLStatement(sql, engine="postgresql") + statement.apply_rls( + None, + None, + { + Table("some_table"): [ + "some_table.tenant_id = 42", + "some_table.region = 'US'", + ], + }, + RLSMethod.AS_PREDICATE_SPLICE, + ) + assert statement.format() == ( + "SELECT * FROM some_table " + "WHERE some_table.tenant_id = 42 AND some_table.region = 'US' " + "AND (status = 'open')" + ) + + def test_rls_predicate_splice_string_predicates_skip_parse() -> None: """ Splice mode accepts predicate strings directly — no ``parse_predicate`` is