CTTY commented on code in PR #3000:
URL: https://github.com/apache/iceberg-rust/pull/3000#discussion_r3825424017
##########
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:
Would using `Arc<dyn SessionCatalog>` directly here block users from using
non-session catalog?
I generally agree that we should have only one abstraction between engine
and the iceberg. Whether we should
1) use `SessionBoundCatalog` here, or
2) a new layer `trait DataFusionCatalog` that contains the logic to resolve
datafusion `Session`
depends on whether there exist a default way to resolve datafusion context
that can cover major area of iceberg use cases, which could be embedded into
`SessionBoundCatalog`. Using `SessionCatalog` directly here feels too limiting
to me.
--
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]
