-
Notifications
You must be signed in to change notification settings - Fork 239
feat: Add refresh
to get get updated TableMetadata
#1154
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
base: main
Are you sure you want to change the base?
Conversation
Is this on track with what we want? cc @liurenjie1024 @Fokko @ZENOTME |
@jonathanc-n This is the very first step, and we indeed want this. I don't think this is the actual conflict detection. When between the |
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.
Thanks @jonathanc-n! I think this PR can be more about add refresh op for table. And as I describe below, it may more appropriate to delay the transaction part after #949.
@@ -127,12 +127,14 @@ impl<'a> Transaction<'a> { | |||
} | |||
|
|||
/// Creates a fast append action. | |||
pub fn fast_append( | |||
pub async fn fast_append( |
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.
According to iceberg-java, looks like we don't need to refresh here. The lifecycle of a transaction looks like this:
- create transaction and get current table
- apply action to current table
- apply action to current table
- commit transaction
- refresh and get the latest table, see: https://github.com/apache/iceberg/blob/c661a71091e496393c743ddd879d9e1a0f2747b2/core/src/main/java/org/apache/iceberg/BaseTransaction.java#L503
- reapply all update before and commit to catalog
I think can delay this part after #949. cc @Fokko @liurenjie1024
refresh
to get get update TableMetadata
refresh
to get get update TableMetadata
refresh
to get get updated TableMetadata
Is there an easy way to be able to create dummy catalog instances and pass them into new table instances for testing? |
Which issue does this PR close?
What changes are included in this PR?
Added basic conflict detection. The current idea is that before commit we can use catalog to refresh the table state before commit to compare with the base metadata.