Skip to content

Conversation

@CTTY
Copy link
Collaborator

@CTTY CTTY commented Jan 13, 2026

Which issue does this PR close?

What changes are included in this PR?

  • Support a CREATE EXTERNAL TABLE with catalog so the table is writable

Are these changes tested?

Added some uts and sqllogictests

@CTTY CTTY marked this pull request as ready for review January 14, 2026 01:45
Copy link
Contributor

@liurenjie1024 liurenjie1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @CTTY for this pr, just finished first round of review, it looks great!

let table_ident = TableIdent::new(namespace.clone(), name.clone());

// Check if table exists before attempting to load
if !catalog
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.");
}


# Create external table with bare name (uses "default" namespace)
statement ok
CREATE EXTERNAL TABLE ns_table STORED AS ICEBERG LOCATION ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test for partitioned table?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support CREATE EXTERNAL TABLE backed by a Catalog with DataFusion

2 participants