databricks-sql-python icon indicating copy to clipboard operation
databricks-sql-python copied to clipboard

Connection without specifying catalog name in connection string causes an error

Open RiyadBen opened this issue 2 years ago • 3 comments

Hello, I'm trying to create an engine connection which could manage multiple catalogs at once

connection_uri = "databricks://token:XXXXXX@DB_HOST?http_path=/sql/1.0/warehouses/DWH_ID"
engine = create_engine(connection_uri, future=True)
meta_inspector = inspect(self.engine)

Later when calling for example tables = meta_inspector.get_table_names('test_schema') I get the following error

backend-1  | sqlalchemy.exc.DatabaseError: (databricks.sql.exc.ServerOperationError) Fail to execute the command as the target schema `None.test_schema` is not in the current catalog. Please set the current catalog with 'USE CATALOG None' first.
backend-1  | [SQL: SHOW VIEWS FROM `None`.`test_schema`]

After further inspection I found the current method being called: https://github.com/databricks/databricks-sql-python/blob/62eb1d407ab03fc2c57a6626bec2173dbdd8bbb3/src/databricks/sqlalchemy/base.py#L278

It could be fixed with adding on Line 278 the following: _target_catalog = kwags.get("catalog") or self.catalog and referencing the catalog name in the method get_table_names tables = meta_inspector.get_table_names(schema='test_schema',catalog='test_catalog')

NOTE : I'm avoiding creating a different engine for every catalog due to slow response from Databricks I'm hesitant in opening a PR for this as it's not well tested and I'm not sure if there is an alternative to what i'm trying to achieve , so any feedback would be great !

RiyadBen avatar Dec 21 '23 15:12 RiyadBen

This is an interesting request and makes sense to me.

This suggestion:

_target_catalog = kwargs.get("catalog") or self.catalog

could work, and seems consistent with the interface that SQLAlchemy defines for get_table_names. Since we run the dialect compliance test suite, so long as the ComponentReflectionTest suite still passes with this change then it should be good for our use case. I'm going to consult internally to ensure this is the path we want to take.

Question for you: are their other inspector / reflection methods that we'd like to support this catalog flag?

susodapop avatar Dec 21 '23 16:12 susodapop

Hello @susodapop , Thank you for your reply So far the only method that i've come across that needs catalog flag without major changes is get_view_names

RiyadBen avatar Jan 03 '24 11:01 RiyadBen