wgtmac commented on code in PR #426:
URL: https://github.com/apache/iceberg-cpp/pull/426#discussion_r2641693152
##########
src/iceberg/transaction.cc:
##########
@@ -68,7 +68,12 @@ Status
Transaction::Apply(std::vector<std::unique_ptr<TableUpdate>> updates) {
last_update_committed_ = true;
Review Comment:
I think the name `last_update_committed_` might confuse readers a little bit
because it only means whether previous created pending update has called commit
to apply changes to the transaction. We use this flag to make sure updates are
created and committed sequentially to make the state of transaction simpler.
Since it has nothing to do with the transaction object itself, I'm hesitant to
set `last_update_committed_` to false when transaction.Commit fails. Perhaps we
should rename it to `last_update_applied` or something more precise. WDYT?
--
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]
