fix: review comments

Author: zhjwpku

src/iceberg/test/CMakeLists.txt

@@ -178,7 +178,6 @@ if(ICEBERG_BUILD_BUNDLE)
                    SOURCES
                    expire_snapshots_test.cc
                    fast_append_test.cc
-                   set_snapshot_test.cc
                    snapshot_manager_test.cc
                    transaction_test.cc
                    update_location_test.cc

src/iceberg/test/set_snapshot_test.cc

@@ -426,11 +426,8 @@ Transaction::NewUpdateSnapshotReference() {
 }
 
 Result<std::shared_ptr<SnapshotManager>> Transaction::NewSnapshotManager() {
-  ICEBERG_ASSIGN_OR_RAISE(std::shared_ptr<SnapshotManager> snapshot_manager,
-                          SnapshotManager::Make(shared_from_this()));
   // SnapshotManager has its own commit logic, so it is not added to the pending updates.
-  // This differs from the Java implementation.
-  return snapshot_manager;
+  return SnapshotManager::Make(shared_from_this());
 }
 
 }  // namespace iceberg

src/iceberg/transaction.cc

@@ -94,17 +94,9 @@ class ICEBERG_EXPORT Transaction : public std::enable_shared_from_this<Transacti
   /// changes.
   Result<std::shared_ptr<UpdateLocation>> NewUpdateLocation();
 
-  /// \brief Create a new SetSnapshot to set the current snapshot or rollback to a
-  /// previous snapshot and commit the changes.
-  Result<std::shared_ptr<SetSnapshot>> NewSetSnapshot();
-
   /// \brief Create a new FastAppend to append data files and commit the changes.
   Result<std::shared_ptr<FastAppend>> NewFastAppend();
 
-  /// \brief Create a new UpdateSnapshotReference to update snapshot references (branches
-  /// and tags) and commit the changes.
-  Result<std::shared_ptr<UpdateSnapshotReference>> NewUpdateSnapshotReference();
-
   /// \brief Create a new SnapshotManager to manage snapshots.
   Result<std::shared_ptr<SnapshotManager>> NewSnapshotManager();
 
@@ -114,6 +106,14 @@ class ICEBERG_EXPORT Transaction : public std::enable_shared_from_this<Transacti
 
   Status AddUpdate(const std::shared_ptr<PendingUpdate>& update);
 
+  /// \brief Create a new SetSnapshot to set the current snapshot or rollback to a
+  /// previous snapshot and commit the changes.
+  Result<std::shared_ptr<SetSnapshot>> NewSetSnapshot();
+
+  /// \brief Create a new UpdateSnapshotReference to update snapshot references (branches
+  /// and tags) and commit the changes.
+  Result<std::shared_ptr<UpdateSnapshotReference>> NewUpdateSnapshotReference();
+
   /// \brief Apply the pending changes to current table.
   Status Apply(PendingUpdate& updates);
 
@@ -132,6 +132,7 @@ class ICEBERG_EXPORT Transaction : public std::enable_shared_from_this<Transacti
 
  private:
   friend class PendingUpdate;
+  friend class SnapshotManager;
 
   // The table that this transaction will update.
   std::shared_ptr<Table> table_;