apache / iceberg-rust

Apache Iceberg
https://rust.iceberg.apache.org/
Apache License 2.0
604 stars 135 forks source link

Add apply interface in transaction #596

Open ZENOTME opened 2 weeks ago

ZENOTME commented 2 weeks ago

This will work for now but might get problematic later on. Just a heads up.

An important concept for Iceberg is to stack snapshots in a single commit. For example, now with append being added in this PR, we can easily add support for truncate. This would be a delete operation where all the data is being dropped, and then just an append.

_Originally posted by @Fokko in https://github.com/apache/iceberg-rust/pull/349#discussion_r1579449633_

Transaction should be able reflect the update in time. According to pyiceberg, we can provide a apply interface to update the table metedata.

ZENOTME commented 2 weeks ago

Exactly. Internally you want to stack the changes together. For example, within a single transaction, you add a new field and then write the data, then the latest schema should be taken into account. We do this in PyIceberg here: https://github.com/apache/iceberg-python/blob/03a0d65ac05d556d0815e61a016effc2b8993702/pyiceberg/table/__init__.py#L715

We can implement the update_table_metadata based on https://github.com/apache/iceberg-rust/pull/587. cc @liurenjie1024 @Xuanwo @Fokko @c-thiel

liurenjie1024 commented 2 weeks ago

Hi, @ZENOTME Could you elaborate on this? I'm kind of confusing about the proposal.

ZENOTME commented 2 weeks ago

Hi, @ZENOTME Could you elaborate on this? I'm kind of confusing about the proposal.

For now, transaction can't reflect the update in time so we can stack them together. e.g.

// table is a v1 table
let tx = Transaction(table);
// This will end up sending two UpgradeFormatVersion into catalog
tx.upgrade_table_version().unwrap().
   .upgrade_table_version().unwrap().commit()

But In pyiceberg, above behaviour will only send one UpgradeFormatVersion and the second one will see that the metadata of table has been updated. The update will be apply into local medata and reflect the change first.

liurenjie1024 commented 1 week ago

We have check to avoid such duplicated case. For metastore tables, it's supposed to apply transaction actions in local, and update metastore pointer. For rest catalog, it should be sent to rest catalog server.

ZENOTME commented 1 week ago

We have check to avoid such duplicated case. For metastore tables, it's supposed to apply transaction actions in local, and update metastore pointer. For rest catalog, it should be sent to rest catalog server.

Why for rest catalog, it should be sent to rest catalog server.🤔 According to API from pyiceberg, it seems possible to create a transaction without auto commit , which means that we also can apply transaction actions in local for rest catalog(do I miss something here)