-
Notifications
You must be signed in to change notification settings - Fork 344
feat(datafusion): support metadata tables for Datafusion #879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d25b534 to
ecf8729
Compare
| [table_name.clone()] | ||
| .into_iter() | ||
| .chain(MetadataTableType::all_types().map(|metadata_table_name| { | ||
| format!("{}${}", table_name.clone(), metadata_table_name.as_str()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The $ syntax follows Flink https://iceberg.apache.org/docs/1.5.2/flink-queries/#inspecting-tables instead of . from Spark, because 4-level catalog.schema.table.metadata is not supported in Datafusion.
| expect![[r#" | ||
| committed_at: PrimitiveArray<Timestamp(Millisecond, Some("+00:00"))> | ||
| [ | ||
| ], | ||
| snapshot_id: PrimitiveArray<Int64> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the datafusion test cases don't have any data present so this is empty
ecf8729 to
726b0fc
Compare
kevinjqliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a rust rookie, this generally LGTM. glad we're exposing these metadata tables using TableProviders
4886b8e to
90abb68
Compare
90abb68 to
0b0af5d
Compare
|
Sorry, closed by mistake. |
0b0af5d to
88b8b48
Compare
90402b2 to
37a79ad
Compare
Signed-off-by: xxchan <[email protected]>
37a79ad to
f830d17
Compare
Signed-off-by: xxchan <[email protected]>
This is resolved (after #1135) and the changes already applied in the PR. This may also be nice to be included in the CLI #1143 😄 |
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
…etadata Signed-off-by: xxchan <[email protected]>
|
kindly ping @liurenjie1024 @Xuanwo @Fokko |
Signed-off-by: xxchan <[email protected]>
|
kindly ping @liurenjie1024 @Xuanwo @Fokko @sdd |
Xuanwo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM, thank you @xxchan for working on this!
|
@Xuanwo can we have this merged? 👀 |
I didn't see other concerns so far. Let's move. |
Use the newly added metadata tables #823 to support this feature in Datafusion engine to demonstrate its usage.
This may also potentially help design and stablize the interface of metadata tables in the
icebergcrate.