feat(datafusion): Support CREATE EXTERNAL TABLE with catalog#2022
feat(datafusion): Support CREATE EXTERNAL TABLE with catalog#2022CTTY wants to merge 10 commits intoapache:mainfrom
Conversation
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @CTTY for this pr, just finished first round of review, it looks great!
|
|
||
| # Create external table with bare name (uses "default" namespace) | ||
| statement ok | ||
| CREATE EXTERNAL TABLE ns_table STORED AS ICEBERG LOCATION '' |
There was a problem hiding this comment.
Can we have a test for partitioned table?
| @@ -96,14 +114,21 @@ impl DataFusionEngine { | |||
| // Create partitioned test table (unpartitioned tables are now created via SQL) | |||
| Self::create_partitioned_table(&catalog, &namespace).await?; | |||
There was a problem hiding this comment.
We should remove this one.
|
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: If the When the Due to the complexity, I think a better approach would be to rename exisiting |
| { | ||
| return Err(to_datafusion_error(Error::new( | ||
| ErrorKind::TableNotFound, | ||
| format!("Table '{table_ident}' not found in catalog"), |
There was a problem hiding this comment.
| format!("Table '{table_ident}' not found in catalog"), | |
| format!("Table '{table_ident}' not found in iceberg catalog"), |
Which issue does this PR close?
CREATE EXTERNAL TABLEbacked by a Catalog with DataFusion #2021What changes are included in this PR?
CREATE EXTERNAL TABLEwith catalog so the table is writableAre these changes tested?
Added some uts and sqllogictests