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

xuanwo 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 9fa3776cb fix: Keep snapshot log on replace (#1896)
9fa3776cb is described below

commit 9fa3776cbd73159809e1c91ea1904ff9449d7a3d
Author: Christian <[email protected]>
AuthorDate: Fri Dec 5 10:27:54 2025 +0100

    fix: Keep snapshot log on replace (#1896)
    
    ## Which issue does this PR close?
    
    Fixes remove_ref() to preserve snapshot log when removing MainBranch
    reference during CREATE OR REPLACE TABLE operations. Previously cleared
    entire snapshot history, causing testReplaceTableKeepsSnapshotLog RCK
    test to fail.
    
    Related Go Issue: https://github.com/apache/iceberg-go/pull/638
    
    Java also does not clear the log:
    
    
https://github.com/apache/iceberg/blob/16e84356dae1975fa04d8c3ecce30a90df18ca9f/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1342-L1352
    
    
    ## What changes are included in this PR?
    
    - Do not clear `snapshot_log` if ref to `main` branch is removed
    
    ## Are these changes tested?
    
    Yes
    
    ---------
    
    Signed-off-by: Xuanwo <[email protected]>
    Co-authored-by: Xuanwo <[email protected]>
---
 crates/iceberg/src/spec/table_metadata_builder.rs | 68 ++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/crates/iceberg/src/spec/table_metadata_builder.rs 
b/crates/iceberg/src/spec/table_metadata_builder.rs
index 6b8ce1e6a..eee4fec34 100644
--- a/crates/iceberg/src/spec/table_metadata_builder.rs
+++ b/crates/iceberg/src/spec/table_metadata_builder.rs
@@ -572,7 +572,6 @@ impl TableMetadataBuilder {
     pub fn remove_ref(mut self, ref_name: &str) -> Self {
         if ref_name == MAIN_BRANCH {
             self.metadata.current_snapshot_id = None;
-            self.metadata.snapshot_log.clear();
         }
 
         if self.metadata.refs.remove(ref_name).is_some() || ref_name == 
MAIN_BRANCH {
@@ -2237,6 +2236,73 @@ mod tests {
         assert_eq!(result.metadata.current_snapshot().unwrap().snapshot_id(), 
2);
     }
 
+    #[test]
+    fn test_remove_main_ref_keeps_snapshot_log() {
+        let builder = builder_without_changes(FormatVersion::V2);
+
+        let snapshot = Snapshot::builder()
+            .with_snapshot_id(1)
+            .with_timestamp_ms(builder.metadata.last_updated_ms + 1)
+            .with_sequence_number(0)
+            .with_schema_id(0)
+            .with_manifest_list("/snap-1.avro")
+            .with_summary(Summary {
+                operation: Operation::Append,
+                additional_properties: HashMap::from_iter(vec![
+                    (
+                        "spark.app.id".to_string(),
+                        "local-1662532784305".to_string(),
+                    ),
+                    ("added-data-files".to_string(), "4".to_string()),
+                    ("added-records".to_string(), "4".to_string()),
+                    ("added-files-size".to_string(), "6001".to_string()),
+                ]),
+            })
+            .build();
+
+        let result = builder
+            .add_snapshot(snapshot.clone())
+            .unwrap()
+            .set_ref(MAIN_BRANCH, SnapshotReference {
+                snapshot_id: 1,
+                retention: SnapshotRetention::Branch {
+                    min_snapshots_to_keep: Some(10),
+                    max_snapshot_age_ms: None,
+                    max_ref_age_ms: None,
+                },
+            })
+            .unwrap()
+            .build()
+            .unwrap();
+
+        // Verify snapshot log was created
+        assert_eq!(result.metadata.snapshot_log.len(), 1);
+        assert_eq!(result.metadata.snapshot_log[0].snapshot_id, 1);
+        assert_eq!(result.metadata.current_snapshot_id, Some(1));
+
+        // Remove the main ref
+        let result_after_remove = result
+            .metadata
+            .into_builder(Some(
+                
"s3://bucket/test/location/metadata/metadata2.json".to_string(),
+            ))
+            .remove_ref(MAIN_BRANCH)
+            .build()
+            .unwrap();
+
+        // Verify snapshot log is kept even after removing main ref
+        assert_eq!(result_after_remove.metadata.snapshot_log.len(), 1);
+        assert_eq!(result_after_remove.metadata.snapshot_log[0].snapshot_id, 
1);
+        assert_eq!(result_after_remove.metadata.current_snapshot_id, None);
+        assert_eq!(result_after_remove.changes.len(), 1);
+        assert_eq!(
+            result_after_remove.changes[0],
+            TableUpdate::RemoveSnapshotRef {
+                ref_name: MAIN_BRANCH.to_string()
+            }
+        );
+    }
+
     #[test]
     fn test_set_branch_snapshot_creates_branch_if_not_exists() {
         let builder = builder_without_changes(FormatVersion::V2);

Reply via email to