aminghadersohi opened a new pull request, #45681: URL: https://github.com/apache/superset/pull/45681
### SUMMARY Avoid inspecting the analytic database's default schema when a virtual dataset already supplies a schema. The RLS changes in https://github.com/apache/superset/commit/f3e620cd0fc9be78387db2ef9fb8f60b08d9bac8 added an unconditional `get_default_schema()` call in both `SqlaTable.get_extra_cache_keys()` and `ExploreMixin.get_from_clause()`. The 6.0 branch contains the equivalent cherry-pick https://github.com/apache/superset/commit/b19372fe16b7d5b06fe36e59e3bff7804059e788; 5.0 lacks the cache-key helper call. `QueryContextProcessor.get_df_payload_result()` computes the cache key before reading the result cache. The default-schema lookup uses `BaseEngineSpec.get_default_schema()` → `Database.get_inspector()` → SQLAlchemy inspection, opening an analytic connection even when its result is discarded by `self.schema or default_schema`. A cache miss repeats the discarded lookup when building the virtual FROM clause. Snowflake inherits that default-schema implementation. Short-circuit both lookups on `self.schema`. Keep the fallback, RLS predicate collection, enforcement, and failure handling intact. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF No UI change. Measured DBAPI connections in the regression test (SQLite/NullPool, configured schema): | Request | Before | After | | --- | ---: | ---: | | Cache hit | 1 | 0 | | Cache miss | 3 | 1 | These measurements reproduce redundant connections, not the reported latency or connection multiplier in a Snowflake deployment. ### TESTING INSTRUCTIONS `test_virtual_dataset_avoids_schema_connections` exercises the real dataframe acquisition, SQL generation, and query execution paths with a mocked result-cache boundary and underlying-dataset lookup. It counts actual DBAPI connections, with/without underlying RLS and with qualified/unqualified SQL. It also checks the RLS cache-key contribution and filtered query results. Before the production change: **8 failed, 2 passed**. After the change: **10 passed**. The two passing cases cover retained default-schema fallback for null/empty schemas. Run from the repository root: ```bash PYTHONPATH=.:superset-core/src pytest -q \ tests/unit_tests/connectors/sqla/virtual_dataset_connections_test.py \ tests/unit_tests/connectors/sqla/models_test.py \ tests/unit_tests/utils/rls_test.py \ tests/unit_tests/models/test_double_rls_virtual_dataset.py \ tests/unit_tests/models/helpers_rls_rollback_test.py \ tests/unit_tests/common/test_query_context_processor.py \ -m 'not manual and not llm_compliance' --disable-warnings ``` Result: **221 passed**. Additional neighbours (`tests/unit_tests/models/helpers_test.py`, `tests/unit_tests/sql/parse_tests.py`, `tests/unit_tests/common/test_query_actions.py`): **1749 passed, 2 skipped, 8 failed** on both unchanged production files and this change, with the identical failure list. The failures are the missing optional `sqlalchemy_bigquery` import and DB2/StarRocks parser cases. They are not included in this change. Staged-file `pre-commit run` and `pre-commit run mypy`: passed. ### ADDITIONAL INFORMATION Sibling paths checked: - Chart data, Explore SQL generation, dashboard/native-filter chart requests, table/pivot queries, grouping-set fallback, samples, and drill-detail use the shared cache-key/query-construction paths above. - `ExploreMixin.values_for_column()` and `validate_expression()` call the same `get_from_clause()`; both receive the schema short-circuit. Their explicit execution connections remain necessary. - `get_sqla_query()` identifier quoting and `Database.compile_sqla_query()` use engines for dialect/compilation, without connecting in this reproduction. `Database.get_dialect()` constructs a dialect without an engine or connection. Snowflake's default-catalog lookup reads the URL. SQLScript parsing itself does not connect. - `utils.rls.find_datasets()` probes the default schema only to disambiguate an existing null-schema dataset match; that RLS-required fallback remains. Schema-less virtual datasets also retain default-schema introspection. Template macros that explicitly access the database are outside this optimization. - `security.manager.get_schemas_accessible_by_user()` and underqualified-table authorization need default-schema resolution; they are not the redundant virtual-dataset lookups. Annotation datasource queries access the metadata database rather than the analytic database. No live Snowflake/dashboard timing validation was performed. No feature flags, migrations, or public API changes. -- 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]
