liurenjie1024 commented on PR #2022: URL: https://github.com/apache/iceberg-rust/pull/2022#issuecomment-3781936786
After second thought, I still fee that the logic of creating external table in datafusion is a little confusion. IIUC, the calling stack of creating external table is like following: ``` let table_provider = table_provier_factory.create(...); schema_provider.register(table_name, table_provider); ``` If the `schema_provider` is non iceberg schema provider, for example `MemorySchemaProvider`, the logic in this pr is absolute correct. But loading a catalog managed iceberg table to put it in non iceberg catalog/schema seem odd to me. When the `schema_provider` is an iceberg schema provider, which is encapsulated by iceberg catalog provider, things get complicated. I think in this case we should create a table rather than simply loading from catalog, which always return error(it has been checked in `schema_provider`). Due to the complexity, I think a better approach would be to rename exisiting `TableProviderFactory` to `StaticTableProviderFactory`, which only loads table from a metadata location. And we create a new `CatalogTableProviderFactory`, which is backed an iceberg catalog. This `CatalogTableProviderFactory` is supposed to be used with `IcebergCatalogProvider`, `IcebergSchemaProvider` together, which provides fully functional table factory, even creating a new table which doesn't exists yet. -- 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]
