CTTY commented on code in PR #2022:
URL: https://github.com/apache/iceberg-rust/pull/2022#discussion_r2709908108
##########
crates/integrations/datafusion/src/table/table_provider_factory.rs:
##########
@@ -115,21 +184,48 @@ impl TableProviderFactory for IcebergTableProviderFactory
{
check_cmd(cmd).map_err(to_datafusion_error)?;
let table_name = &cmd.name;
- let metadata_file_path = &cmd.location;
- let options = &cmd.options;
-
let table_name_with_ns = complement_namespace_if_necessary(table_name);
- let table = create_static_table(table_name_with_ns,
metadata_file_path, options)
- .await
- .map_err(to_datafusion_error)?
- .into_table();
-
- let provider = IcebergStaticTableProvider::try_new_from_table(table)
- .await
- .map_err(to_datafusion_error)?;
-
- Ok(Arc::new(provider))
+ match &self.catalog {
+ Some(catalog) => {
+ // Catalog-backed: create IcebergTableProvider
+ let (namespace, name) =
parse_table_reference(&table_name_with_ns);
+ let table_ident = TableIdent::new(namespace.clone(),
name.clone());
+
+ // Check if table exists before attempting to load
+ if !catalog
Review Comment:
Do you mean that we should check if the table already exists in the
`IcebergCatalogProvider`/`SchemaProvider`? I think that verification is done
when datafusion is calling `register_table` after
`TableProviderFactory::create`:
https://github.com/apache/datafusion/blob/a9e6d4be4a63c2180814a627d3b45cd0adf5b61c/datafusion/core/src/execution/context/mod.rs#L803
I don't think we have a way to verify that within `TableProviderFactory`
--
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]
