Skip to content

Commit 5afbffd

Browse files
committed
fix: align Apply() behavior with Java - don't revert last_update_committed_ on Transaction::Commit() failure
In Java's BaseTransaction, the hasLastOpCommitted flag tracks whether PendingUpdate.commit() was called (which applies updates to transaction metadata), NOT whether Transaction.commitTransaction() succeeded in persisting to the catalog. Once PendingUpdate.commit() is called and updates are applied to the transaction's metadata, hasLastOpCommitted is set to true and stays true even if Transaction.commitTransaction() fails later. The previous implementation incorrectly reverted last_update_committed_ to false when auto-commit failed. This mixed two separate concerns: 1. Whether the pending update applied its changes (tracked by the flag) 2. Whether the transaction persisted to catalog (separate concern) This fix: - Removes the flag reversion on Commit() failure - Matches Java behavior where the flag only tracks PendingUpdate state - Simplifies Apply() by using ICEBERG_RETURN_UNEXPECTED Reference: BaseTransaction.java in Apache Iceberg - hasLastOpCommitted set to true in TransactionTableOperations.commit() - Never reverted based on Transaction.commitTransaction() result
1 parent 25c70c2 commit 5afbffd

File tree

1 file changed

+1
-5
lines changed

1 file changed

+1
-5
lines changed

src/iceberg/transaction.cc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,7 @@ Status Transaction::Apply(std::vector<std::unique_ptr<TableUpdate>> updates) {
6868
last_update_committed_ = true;
6969

7070
if (auto_commit_) {
71-
auto result = Commit();
72-
if (!result.has_value()) {
73-
last_update_committed_ = false;
74-
return std::unexpected(result.error());
75-
}
71+
ICEBERG_RETURN_UNEXPECTED(Commit());
7672
}
7773

7874
return {};

0 commit comments

Comments
 (0)