liurenjie1024 commented on code in PR #2022:
URL: https://github.com/apache/iceberg-rust/pull/2022#discussion_r2710609485
##########
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:
No, I mean when we hit this code path, we should actually create a new table
rather than loading an existing one.
##########
crates/sqllogictest/src/engine/datafusion.rs:
##########
@@ -96,14 +114,21 @@ impl DataFusionEngine {
// Create partitioned test table (unpartitioned tables are now created
via SQL)
Self::create_partitioned_table(&catalog, &namespace).await?;
Review Comment:
We should remove this one.
--
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]
