aminghadersohi commented on code in PR #45681:
URL: https://github.com/apache/superset/pull/45681#discussion_r4380947685
##########
superset/models/helpers.py:
##########
@@ -3994,6 +3995,24 @@ def get_rendered_sql(
def text(self, clause: str) -> TextClause:
return self.db_engine_spec.get_text_clause(clause)
+ def _get_rls_schema(self) -> str:
Review Comment:
I corrected the execution-schema RLS behavior in
57f71df01abfd0b8fba7445ad28f004e349b188e. Rather than resolving the schema
eagerly at every caller, `validate_adhoc_subquery` calls the shared
`get_rls_schema` resolver immediately before `apply_rls`, after detecting a
subquery. This reaches the connection-default table on non-dynamic engines and
avoids schema-discovery connections for ordinary expressions. Scalar expression
roots also get a SELECT wrapper so scope traversal finds their tables.
Regression tests in
`tests/unit_tests/connectors/sqla/virtual_dataset_connections_test.py`:
- `test_adhoc_subquery_rls_uses_connection_schema`: actual SQLite results
and injected predicate counts for adhoc columns, arithmetic, metrics, WHERE,
HAVING, orderby and filter-column subqueries, on both SqlaTable and Query
datasources.
- `test_adhoc_subquery_rls_cache_uses_connection_schema`: expression-only
table references change the cache key when their RLS rule changes.
- `test_expression_validation_rls_uses_connection_schema`: stored, rendered,
save-time and select-list-fragment validators.
- `test_stored_subquery_rls_uses_connection_schema`: stored and
Jinja-rendered columns/metrics, for execution and cache keys.
- `test_adhoc_subquery_rls_schema_discovery_fails_closed` and
`test_stored_subquery_rls_dialect_fallback_fails_closed`: discovery failures
and rewrites available only in a fallback dialect cannot silently discard RLS.
I reran these six groups in Docker with current tests and production source
from 5c707e415e9db47ddaa89e40503fbe656c882feb: **48 failed, 149 deselected**.
With production source at cc34e0f9d6f180910b29b9b511f6dae51925ec7e: **48
passed, 149 deselected**. Commands: `python -m pytest -p no:cacheprovider
tests/unit_tests/connectors/sqla/virtual_dataset_connections_test.py -q -k
'<the six test names above joined with or>' -m 'not manual and not
llm_compliance'`.
Remaining: the cited callers still pass `self.schema` (or `schema`) as the
configured-schema input; I did not perform the literal caller-by-caller
substitution with `_get_rls_schema()`. The shared validator resolves that input
to the execution schema before RLS lookup, as the failing-before/passing-after
cases demonstrate.
Siblings checked:
- superset/models/helpers.py:4197 — fixed in
57f71df01abfd0b8fba7445ad28f004e349b188e
- superset/models/helpers.py:5133 — fixed in
57f71df01abfd0b8fba7445ad28f004e349b188e
- superset/models/helpers.py:5214 — fixed in
57f71df01abfd0b8fba7445ad28f004e349b188e
- superset/models/helpers.py:5274 — fixed in
57f71df01abfd0b8fba7445ad28f004e349b188e
- superset/models/helpers.py:6003 — fixed in
57f71df01abfd0b8fba7445ad28f004e349b188e
- superset/models/helpers.py:6012 — fixed in
57f71df01abfd0b8fba7445ad28f004e349b188e
- superset/connectors/sqla/models.py:1318 — fixed in
57f71df01abfd0b8fba7445ad28f004e349b188e
Those are the original line numbers in your comment. At this head the six
helper callers are at 4353, 5294, 5375, 5435, 6164 and 6173; the save-time
validator is at connectors/sqla/models.py:1319. They converge on helpers.py:504
(including its stored/rendered wrappers), which resolves through
utils/rls.py:58. Expression-only cache collection uses the same resolver
through `_get_query_rls_cache_keys`; d143dac1160081327b63028da3fbd448ca9f4f9e
and a121abfb2b17aa4fd8aab1c0d56275252152229d retain that coverage without
building full chart queries for RLS keys.
--
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]
