DerGut commented on code in PR #3000:
URL: https://github.com/apache/iceberg-rust/pull/3000#discussion_r3842639441
##########
crates/integrations/datafusion/src/table/mod.rs:
##########
@@ -80,18 +83,21 @@ impl IcebergTableProvider {
/// Loads the table once to get the initial schema, then stores the catalog
/// reference for future metadata refreshes on each operation.
pub(crate) async fn try_new(
- catalog: Arc<dyn Catalog>,
+ catalog_access: CatalogAccess,
Review Comment:
Since we are now using the `SessionBindingCatalogAdapter` to back all
providers, I added ways to build it from `dyn Catalog` and `dyn
SessionCatalog`. From a `SessionCatalog`, it's a [trivial
construction](https://github.com/DerGut/iceberg-rust/blob/5eecdff089b53aceae0ba9cd738effee609064cd/crates/integrations/datafusion/src/catalog_access.rs#L47)
from a provided, bound `SessionContext` and a `dyn SessionCatalog`. From a
`dyn Catalog`, I'm [first creating a `dyn SessionCatalog` via the internal
adapter](https://github.com/DerGut/iceberg-rust/blob/5eecdff089b53aceae0ba9cd738effee609064cd/crates/integrations/datafusion/src/catalog_access.rs#L54)
`SessionDroppingCatalogAdapter`, then use it to construct one with an empty
`SessionContext`.
This means the current implementation offers two pub APIs, one `dyn
Catalog`-based and one `dyn SessionCatalog` based.
Dropping the old `dyn Catalog` API would mean we'd make the internal
`SessionDroppingCatalogAdapter` public.
--
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]
