liurenjie1024 commented on code in PR #2022:
URL: https://github.com/apache/iceberg-rust/pull/2022#discussion_r2696508048
##########
crates/sqllogictest/testdata/slts/df_test/create_table.slt:
##########
@@ -88,3 +92,79 @@ SELECT * FROM default.default.nullable_table
----
1 Value
2 NULL
+
+# =============================================================================
+# Test CREATE EXTERNAL TABLE (catalog-backed table loading)
+# =============================================================================
+
+# First, create a table in the catalog that we'll load via CREATE EXTERNAL
TABLE
+statement ok
+CREATE TABLE default.default.source_table (id INT NOT NULL, data STRING)
+
+# Insert some data into the source table
+query I
+INSERT INTO default.default.source_table VALUES (1, 'first'), (2, 'second')
+----
+2
+
+# Verify source table data
+query IT rowsort
+SELECT * FROM default.default.source_table
+----
+1 first
+2 second
+
+# Create an external table that loads the existing catalog table
+# The table name in CREATE EXTERNAL TABLE is used to look up the table in the
catalog
+# LOCATION is ignored when using catalog-backed factory
+statement ok
+CREATE EXTERNAL TABLE source_table STORED AS ICEBERG LOCATION ''
+
+# Query the external table - should see the same data as the catalog table
+query IT rowsort
+SELECT * FROM source_table
+----
+1 first
+2 second
+
+# Insert data via the external table (catalog-backed providers support writes)
+query I
+INSERT INTO source_table VALUES (3, 'third')
+----
+1
+
+# Verify the insert worked - query via external table
+query IT rowsort
+SELECT * FROM source_table
+----
+1 first
+2 second
+3 third
+
+# Verify the insert is visible via the original catalog table path
+query IT rowsort
+SELECT * FROM default.default.source_table
+----
+1 first
+2 second
+3 third
+
+# Test CREATE EXTERNAL TABLE with namespace-qualified name
+statement ok
+CREATE TABLE default.default.ns_table (value INT NOT NULL)
+
+query I
+INSERT INTO default.default.ns_table VALUES (100), (200)
+----
+2
+
+# Create external table with bare name (uses "default" namespace)
+statement ok
+CREATE EXTERNAL TABLE ns_table STORED AS ICEBERG LOCATION ''
Review Comment:
Can we have a test for partitioned table?
##########
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:
When datafusion calls this method to create a table, it already has
deregistered table. But since datafusion allows no-iceberg catalog to register
iceberg table, this check may still miss match. I think we should do is that:
```
if catalog.table.exists(...) {
return Error::new(ErrorKind::DataInvalid, "Table already exists in iceberg
catalog ..., this maybe a misconfig datafusion catalog provider vs iceberg
catalog.");
}
```
--
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]
