(iceberg-rust) branch main updated: fix: snapshot was producing empty summary (#1767)

[liurenjie1024]

This is an automated email from the ASF dual-hosted git repository.

liurenjie1024 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git


The following commit(s) were added to refs/heads/main by this push:
     new 3aa7ade2 fix: snapshot was producing empty summary (#1767)
3aa7ade2 is described below

commit 3aa7ade28e6e56304700c163e1cbfdd8b0069921
Author: Raminder Singh <romi_...@yahoo.co.in>
AuthorDate: Wed Oct 22 15:03:44 2025 +0530

    fix: snapshot was producing empty summary (#1767)
    
    ## Which issue does this PR close?
    
    - Closes #. There was no open issue for this. I noticed this while
    trying out iceberg-rust.
    
    ## What changes are included in this PR?
    
    The first line in the method `SnapshotProducer::write_added_manifest`
    (`let added_data_files = std::mem::take(&mut self.added_data_files);`)
    sets the `self.added_data_files` to an empty vec.
    `SnapshotProducer::write_added_manifest` is called via the call chain
    `SnapshotProducer::commit` -> `SnapshotProducer::manifest_file` ->
    `SnapshotProducer::write_added_manifest`). Hence, if
    `SnapshotProducer::summary` is called after calling
    `SnapshotProducer::manifest_file`, the summary produced was empty due to
    empty `self.added_data_files`.
    
    This PR rearranges the code in `SnapshotProducer::commit` to make sure
    the produced summary is not empty.
    
    ## Are these changes tested?
    
    No new tests have been added for this as there were no previous tests.
---
 crates/iceberg/src/transaction/snapshot.rs | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/crates/iceberg/src/transaction/snapshot.rs 
b/crates/iceberg/src/transaction/snapshot.rs
index a03b8dc4..93dd819d 100644
--- a/crates/iceberg/src/transaction/snapshot.rs
+++ b/crates/iceberg/src/transaction/snapshot.rs
@@ -380,17 +380,8 @@ impl<'a> SnapshotProducer<'a> {
         snapshot_produce_operation: OP,
         process: MP,
     ) -> Result<ActionCommit> {
-        let new_manifests = self
-            .manifest_file(&snapshot_produce_operation, &process)
-            .await?;
-        let next_seq_num = self.table.metadata().next_sequence_number();
-
-        let summary = self.summary(&snapshot_produce_operation).map_err(|err| {
-            Error::new(ErrorKind::Unexpected, "Failed to create snapshot 
summary.").with_source(err)
-        })?;
-
         let manifest_list_path = self.generate_manifest_list_file_path(0);
-
+        let next_seq_num = self.table.metadata().next_sequence_number();
         let mut manifest_list_writer = match 
self.table.metadata().format_version() {
             FormatVersion::V1 => ManifestListWriter::v1(
                 self.table
@@ -408,6 +399,18 @@ impl<'a> SnapshotProducer<'a> {
                 next_seq_num,
             ),
         };
+
+        // Calling self.summary() before self.manifest_file() is important 
because self.added_data_files
+        // will be set to an empty vec after self.manifest_file() returns, 
resulting in an empty summary
+        // being generated.
+        let summary = self.summary(&snapshot_produce_operation).map_err(|err| {
+            Error::new(ErrorKind::Unexpected, "Failed to create snapshot 
summary.").with_source(err)
+        })?;
+
+        let new_manifests = self
+            .manifest_file(&snapshot_produce_operation, &process)
+            .await?;
+
         manifest_list_writer.add_manifests(new_manifests.into_iter())?;
         manifest_list_writer.close().await?;