Skip to content

Fix: Add Proper Catalog Support to Engine Adapter#1559

Merged
eakmanrq merged 1 commit intomainfrom
eakmanrq/add_proper_engine_adapter_catalog_support
Oct 12, 2023
Merged

Fix: Add Proper Catalog Support to Engine Adapter#1559
eakmanrq merged 1 commit intomainfrom
eakmanrq/add_proper_engine_adapter_catalog_support

Conversation

@eakmanrq
Copy link
Contributor

Controversial change to highlight: Removed the field catalog_name and instead let callers provide a TableName which means it can either be a string or exp.Table object.

Added engine adapter methods:

  • get_current_catalog
  • set_current_catalog

New Property: CatalogSupport
This tells the set_catalog decorator what level of catalog support the engine adapter has in order to set the catalog properly. 3 levels:

  • Unsupported
  • Requires Set Catalog
  • Full Support

Requires Set Catalog means that operations cannot be provided with the catalog but instead they need set_current_catalog against the catalog and then they can provide the operation but now with the catalog dropped. This always refers to the target of the operation (ex: if drop schema, the schema being drooped) and we assume reads can happen across catalogs. For engines that don't support reads across catalogs we just consider them Unsupported (ex: Postgres).

Follow up work:

  • Have new default catalog integrate with connector default catalog if it supports it
  • Add Redshift catalog support
    • The challenge with this is that Redshift doesn't allow changing the catalog from within a connection. A new connection needs to be created with the new catalog defined. This is just for creating objects across catalogs and not querying across catalogs

@eakmanrq eakmanrq force-pushed the eakmanrq/add_proper_engine_adapter_catalog_support branch from 0dcf4eb to 3bee118 Compare October 11, 2023 23:08
@eakmanrq eakmanrq force-pushed the eakmanrq/add_proper_engine_adapter_catalog_support branch from 3bee118 to 87976de Compare October 11, 2023 23:38
Copy link
Contributor

@tobymao tobymao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

z

@eakmanrq eakmanrq force-pushed the eakmanrq/add_proper_engine_adapter_catalog_support branch 4 times, most recently from c667bca to a6845f3 Compare October 12, 2023 03:50
@eakmanrq eakmanrq force-pushed the eakmanrq/add_proper_engine_adapter_catalog_support branch 3 times, most recently from fdb8f2d to 5fe9689 Compare October 12, 2023 17:39
@eakmanrq eakmanrq force-pushed the eakmanrq/add_proper_engine_adapter_catalog_support branch from 5fe9689 to 1a386d4 Compare October 12, 2023 18:11
@eakmanrq eakmanrq merged commit c37b0aa into main Oct 12, 2023
@eakmanrq eakmanrq deleted the eakmanrq/add_proper_engine_adapter_catalog_support branch October 12, 2023 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants