[EPIC] Support for appending data to iceberg table.

[liurenjie1024]

What's the feature are you trying to implement?

This issue is used to track implementing appending data to iceberg table. In summary, after we complete all subtasks in this issue, we should be able to execution following insert sql statment in datafusion to insert table to iceberg table:

insert into t value (1, 'a');

For more details about insert statement, please refer to datafusion doc.

Commit path

• Add a method in TableCommit to apply changes to TableMetadata to create a new TableMetadata #1386
• Add retry in Transaction::commit method #1387
• Implement saving TableMetadata to new location. #1388
• Swith metadata pointer to new location for different catalogs(sql, hive, glue, etc) #1389

Writer path

• Implement fanout partitioned data writer.
• Implement non fanout partitioned data writer.
• Implement data writer for non partitioned table.

Datafusion integration

• Implement insert_into method for IcebergTableProvider

Willingness to contribute

I would be willing to contribute to this feature with guidance from the Iceberg Rust community

[jonathanc-n]

Thanks for creating this @liurenjie1024!

[ZENOTME]

Thanks @liurenjie1024 for this! Let's move!

Add a method in TableCommit to apply changes to TableMetadata to create a new TableMetadata

I don't get why we need this. Is this used by the catalog implementation, e.g. SqlCatalog::update_table? Could you elaborate more the commit process and where we need this?

Add retry in Transaction::commit method

Maybe we can track this sub task in #964.

[CTTY]

Hi @liurenjie1024 , thanks for having this!

Would it be ok if I take up the commit path work? I can start working on a POC tomorrow

[liurenjie1024]

I don't get why we need this. Is this used by the catalog implementation, e.g. SqlCatalog::update_table? Could you elaborate more the commit process and where we need this?

Yes. Catalogs uses this new method to generate a new table metadata, and then it points to new table metadata.

#964 duplicates with this issue.

[liurenjie1024]

Hi @liurenjie1024 , thanks for having this!

Would it be ok if I take up the commit path work? I can start working on a POC tomorrow

Cool, thanks!

[hsingh574]

@CTTY @liurenjie1024 I'm wondering about the commit path proposed for the memory catalog in #1381 , which does #1388 and #1389 , but not the other two things. I think the retries aren't needed because the memory catalog uses an explicit lock rather than optimistic concurrency, and it doesn't seem necessary to me to have TableCommit provide a create_new_metadata function rather than applying the updates in the catalog. Thoughts on this?

[liurenjie1024]

@CTTY @liurenjie1024 I'm wondering about the commit path proposed for the memory catalog in #1381 , which does #1388 and #1389 , but not the other two things. I think the retries aren't needed because the memory catalog uses an explicit lock rather than optimistic concurrency, and it doesn't seem necessary to me to have TableCommit provide a create_new_metadata function rather than applying the updates in the catalog. Thoughts on this?

Hi, @hsingh574 Using a lock doesn't mean it can't do optimistic concurrency control. The retry is used to validate that all table requiremens are met.

[hsingh574]

Using a lock doesn't mean it can't do optimistic concurrency control

@liurenjie1024 I don't think it makes sense for multiple MemoryCatalogs to be working on the same table. If thats the case, then all updates to a given table should be serialized by the lock in the MemoryCatalog, so there should be no need to retry because the table metadata cannot have changed after the requirements have been checked once under the lock.