sadpandajoe commented on code in PR #43695:
URL: https://github.com/apache/superset/pull/43695#discussion_r4260350930


##########
superset/db_engine_specs/databricks.py:
##########
@@ -1011,6 +1020,49 @@ def build_sqlalchemy_uri(  # type: ignore
             query=query,
         ).render_as_string(hide_password=False)
 
+    @classmethod
+    def adjust_engine_params(
+        cls,
+        uri: URL,
+        connect_args: dict[str, Any],
+        catalog: str | None = None,
+        schema: str | None = None,
+    ) -> tuple[URL, dict[str, Any]]:
+        adjusted_connect_args = connect_args.copy()
+
+        effective_catalog = (
+            catalog
+            or adjusted_connect_args.get("catalog")
+            or uri.query.get("catalog")
+            or cls.default_catalog

Review Comment:
   Forcing `hive_metastore` whenever no catalog is supplied changes behavior 
for existing connections: in a workspace whose default catalog is `main`, an 
unqualified `SELECT * FROM orders` now resolves against 
`hive_metastore.default.orders`, and a server that reports no multi-catalog 
support can reject the connection handshake because a catalog is now always 
sent. `get_default_catalog()` also can no longer discover the workspace 
default, since its discovery connection already carries the forced catalog. 
Should the fallback be omitted (leave the catalog unset) when none is 
configured, so the server default still applies?



##########
superset/db_engine_specs/databricks.py:
##########
@@ -1011,6 +1020,49 @@ def build_sqlalchemy_uri(  # type: ignore
             query=query,
         ).render_as_string(hide_password=False)
 
+    @classmethod
+    def adjust_engine_params(
+        cls,
+        uri: URL,
+        connect_args: dict[str, Any],
+        catalog: str | None = None,
+        schema: str | None = None,
+    ) -> tuple[URL, dict[str, Any]]:
+        adjusted_connect_args = connect_args.copy()
+
+        effective_catalog = (
+            catalog
+            or adjusted_connect_args.get("catalog")
+            or uri.query.get("catalog")
+            or cls.default_catalog
+        )
+        effective_schema = (
+            schema
+            or adjusted_connect_args.get("schema")
+            or uri.query.get("schema")

Review Comment:
   When only a catalog override is passed (e.g. 
`get_all_schema_names(catalog="main")`), this still carries the URI's database 
as the schema, so a URI ending in `/finance` produces `catalog=main, 
schema=finance`. If `finance` exists only in the original catalog, the 
connection's initial namespace fails before the prequery that switches catalog 
can run, and the schema dropdown for the new catalog never loads. Should the 
URI-database schema fallback only apply when the catalog is not being 
overridden?



-- 
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]

Reply via email to