From 17c03f12f10412294c1d9d475cff62411a5578a7 Mon Sep 17 00:00:00 2001 From: Xinli Shang Date: Sun, 21 Dec 2025 14:52:14 -0800 Subject: [PATCH 1/2] fix(transaction): fix exception safety in Apply() Set last_update_committed_ before calling Commit(), but revert the flag if auto-commit fails. This ensures: 1. Commit() can proceed (it requires last_update_committed_ == true) 2. If commit fails, the flag is reverted to false 3. Transaction state remains consistent on failure --- src/iceberg/transaction.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/iceberg/transaction.cc b/src/iceberg/transaction.cc index ca39ec043..a852710ee 100644 --- a/src/iceberg/transaction.cc +++ b/src/iceberg/transaction.cc @@ -68,7 +68,12 @@ Status Transaction::Apply(std::vector> updates) { last_update_committed_ = true; if (auto_commit_) { - ICEBERG_RETURN_UNEXPECTED(Commit()); + auto result = Commit(); + if (!result.has_value()) { + // Commit failed, revert the flag + last_update_committed_ = false; + return std::unexpected(result.error()); + } } return {}; From fb231c47dfdb45c6785314be6fa5009cd6c9fccc Mon Sep 17 00:00:00 2001 From: Xinli Shang Date: Wed, 24 Dec 2025 07:45:17 -0800 Subject: [PATCH 2/2] fix: align Transaction::Apply() with Java BaseTransaction behavior Match Java's BaseTransaction where hasLastOpCommitted tracks whether PendingUpdate.commit() was called, not Transaction.commitTransaction(). Key changes: - Remove last_update_committed_ reversion on Transaction::Commit() failure - The flag tracks PendingUpdate state (updates applied to metadata_builder_) - Transaction persistence to catalog is a separate concern This aligns with Java BaseTransaction.TransactionTableOperations.commit() at line 645 which sets hasLastOpCommitted=true and never reverts it based on transaction commit results. --- src/iceberg/transaction.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/iceberg/transaction.cc b/src/iceberg/transaction.cc index a852710ee..d19e82690 100644 --- a/src/iceberg/transaction.cc +++ b/src/iceberg/transaction.cc @@ -70,8 +70,6 @@ Status Transaction::Apply(std::vector> updates) { if (auto_commit_) { auto result = Commit(); if (!result.has_value()) { - // Commit failed, revert the flag - last_update_committed_ = false; return std::unexpected(result.error()); } }