aminghadersohi commented on code in PR #45681:
URL: https://github.com/apache/superset/pull/45681#discussion_r4380375748
##########
superset/models/helpers.py:
##########
@@ -4015,7 +4015,9 @@ def get_from_clause(
# Apply RLS filters to virtual dataset SQL to prevent RLS bypass
# For each table referenced in the virtual dataset, apply its RLS
filters
if parsed_script.statements:
- default_schema = self.database.get_default_schema(self.catalog)
+ default_schema = self.schema or self.database.get_default_schema(
Review Comment:
Correction to my reply above: at 57f71df01abfd0b8fba7445ad28f004e349b188e
the adhoc-expression RLS fix holds, but the new cache-key build adds
connections on cache hits. It also does not hold that the chart query is only
built, never executed.
With `ALLOW_ADHOC_SUBQUERY` on, `_get_query_rls_cache_keys` builds the full
chart query on every cache-key computation, including cache hits:
- For a dataset without a schema on a dynamic-schema engine, or any dataset
on a static engine whose default schema needs the inspector, each cache hit
opens 3 connections (4 with an adhoc subquery). At
5c707e415e9db47ddaa89e40503fbe656c882feb and on master it opens 1. The extra
inspector calls come from `get_from_clause` during the build,
`_get_query_rls_cache_keys` and `validate_adhoc_subquery`. This happens even
when no expression contains a subquery.
- On engines without joins or subqueries (Couchbase, Pinot, Solr,
Elasticsearch, Druid without `DRUID_JOINS`), `get_sqla_query` runs a
series-limit chart's prequery through `self.query()`. That now happens on every
cache hit: 1 connection and one executed query, where before there were none.
The 64 `test_virtual_dataset_avoids_schema_connections` cases pass, but each
gives the dataset a schema that needs no discovery, and none uses a series
limit.
What checks out: the 48 cited regression tests fail with `superset/` at
5c707e41 and pass at 57f71df0. Adhoc series-limit metrics, time-grain x-axis
columns and `AS_SUBQUERY` engines also get the predicate in the query and the
cache key.
Not fixed: the cache-hit connection counts above, and the prequery that runs
during cache-key computation on engines without joins.
--
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]
