CTTY commented on code in PR #1400:
URL: https://github.com/apache/iceberg-rust/pull/1400#discussion_r2118723879
##########
crates/catalog/memory/src/catalog.rs:
##########
@@ -277,12 +278,75 @@ impl Catalog for MemoryCatalog {
}
/// Update a table to the catalog.
- async fn update_table(&self, _commit: TableCommit) -> Result<Table> {
+ async fn update_table(&self, commit: TableCommit) -> Result<Table> {
Err(Error::new(
ErrorKind::FeatureUnsupported,
"MemoryCatalog does not currently support updating tables.",
))
}
+
+ async fn commit_table(&self, base: &Table, current: Table) ->
Result<Table> {
Review Comment:
I'm trying to replace `update_table` with `commit_table`, also would love
some feedback on this:
- Do we actually need a `TableCommit` for catalog to commit tables?
- In the `Transaction` we have already staged changes to the
`current_table`[link](https://github.com/apache/iceberg-rust/blob/main/crates/iceberg/src/transaction/mod.rs#L42).
- `Transaction` will need to hold `Vec<TransactionAction>` so tx itself
can rebase updates easily, if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
