ZENOTME commented on PR #1400: URL: https://github.com/apache/iceberg-rust/pull/1400#issuecomment-2945140478
> Hi, @CTTY I think we should seperate our concerns here. Our design could be inspired by java, but it doesn't mean we should blindly copy java's class hierarchy. There are a lot of differences between these two languages. The `T apply()` method in java is quite convenient since java's language elements facilities such complex class hierarchies, but this is not the case for rust. > > I don't think it's a good idea to add an `apply` method in the general purpose `TransactionAction`. `TransactionAction` is used to be stored in `Transaction` so that we could do retry when commit exception, so `commit` is the only method required. In future we may have some type safe trait for actions which may produce new snapshot, but this is not the concern in this pr. +1 for we don't need to apply now. In iceberg-java, the apply can be used for code reuse. E.g. SnapshotProducer implements `apply` and `commit`, and CherryPickOperation inherits from SnapshotProducer, overwrites the `apply` but reuses the `commit`. But we can't do this in Rust. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
