EnxDev commented on code in PR #44650:
URL: https://github.com/apache/superset/pull/44650#discussion_r4104644955
##########
superset/utils/rls.py:
##########
@@ -83,24 +85,39 @@ def apply_rls(
# collect all RLS predicates for all tables in the query
default_catalog = database.get_default_catalog()
- predicates: dict[Table, list[Any]] = {}
- for table in parsed_statement.tables:
- table = table.qualify(catalog=catalog, schema=schema)
- predicates[table] = [
- parsed_statement.parse_predicate(predicate)
- for predicate in get_predicates_for_table(
- table,
- database,
- default_catalog,
- exclude_dataset_id=exclude_dataset_id,
- include_global_guest_rls=include_global_guest_rls,
- )
- if predicate
- ]
-
- has_predicates = any(predicates.values())
- parsed_statement.apply_rls(catalog, schema, predicates, method)
- return has_predicates
+
+ def collect_predicates(include_global: bool) -> dict[Table, list[Any]]:
+ predicates: dict[Table, list[Any]] = {}
+ for table in parsed_statement.tables:
+ table = table.qualify(catalog=catalog, schema=schema)
+ predicates[table] = [
+ parsed_statement.parse_predicate(predicate)
+ for predicate in get_predicates_for_table(
+ table,
+ database,
+ default_catalog,
+ exclude_dataset_id=exclude_dataset_id,
+ include_global_guest_rls=include_global,
+ )
+ if predicate
+ ]
+ return predicates
+
+ predicates = collect_predicates(include_global_guest_rls)
+ # The outer query only constrains the rows that reach it, so a table read
+ # inside a sub-query still gets the global guest rules left to the outer
query.
+ # Only a guest token carries such rules, so other users skip the second
lookup.
+ subquery_predicates = (
+ collect_predicates(True)
Review Comment:
Fair point, a correlated lookup like that is keyed to the virtual dataset's
own rows the same way a join is, so it should get the same treatment.
69067ace73 leaves correlated sub-queries to the outer filter, and your
`lookup.id = a.lid` example is now a test that keeps `org_id` out of the SQL.
Uncorrelated sub-queries such as `(SELECT count(*) FROM b)`, which nothing
outside constrains, still get the rule.
Correlation is detected from columns qualified with an enclosing table's
name or alias. I didn't use sqlglot's `Scope.is_correlated_subquery`: it treats
every unqualified column as external, so it also flags a plain `a.k IN (SELECT
k FROM b)` and would skip it. An unqualified reference counts as local here,
which errs toward applying the rule, so `WHERE id = lid` still gets it. That
tradeoff and the remaining gap (a correlated aggregate over another tenant's
table, same as the join case) are in UPDATING.md.
--
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]
