(iceberg-rust) branch main updated: fix: TableMetadata max sequence number validation (#1167)

[li...@apache.org]

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 d7e2685e fix: TableMetadata max sequence number validation (#1167)
d7e2685e is described below

commit d7e2685e7e64be6b5877356587c2cd847d51b0cb
Author: Christian <ch...@hansetag.com>
AuthorDate: Tue Apr 8 06:18:56 2025 -0700

    fix: TableMetadata max sequence number validation (#1167)
    
    ## What changes are included in this PR?
    Ensures that the sequence number of any snapshot in TableMetadata is
    lower or equal to the `last_sequence_number`.
    
    In Java this happens during indexing:
    
    https://github.com/apache/iceberg/blob/0152075268851d397b60bcb66e5a7c346271dfd9/core/src/main/java/org/apache/iceberg/TableMetadata.java#L842
---
 crates/iceberg/src/spec/table_metadata.rs | 105 +++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 2 deletions(-)

diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs
index 59d69e0f..47ba7823 100644
--- a/crates/iceberg/src/spec/table_metadata.rs
+++ b/crates/iceberg/src/spec/table_metadata.rs
@@ -434,7 +434,7 @@ impl TableMetadata {
         self.validate_chronological_metadata_logs()?;
         // Normalize location (remove trailing slash)
         self.location = self.location.trim_end_matches('/').to_string();
-        self.validate_format_version_specifics()?;
+        self.validate_snapshot_sequence_number()?;
         self.try_normalize_partition_spec()?;
         self.try_normalize_sort_order()?;
         Ok(self)
@@ -547,7 +547,7 @@ impl TableMetadata {
     }
 
     /// Validate that for V1 Metadata the last_sequence_number is 0
-    fn validate_format_version_specifics(&self) -> Result<()> {
+    fn validate_snapshot_sequence_number(&self) -> Result<()> {
         if self.format_version < FormatVersion::V2 && self.last_sequence_number != 0 {
             return Err(Error::new(
                 ErrorKind::DataInvalid,
@@ -558,6 +558,24 @@ impl TableMetadata {
             ));
         }
 
+        if self.format_version >= FormatVersion::V2 {
+            if let Some(snapshot) = self
+                .snapshots
+                .values()
+                .find(|snapshot| snapshot.sequence_number() > self.last_sequence_number)
+            {
+                return Err(Error::new(
+                    ErrorKind::DataInvalid,
+                    format!(
+                        "Invalid snapshot with id {} and sequence number {} greater than last sequence number {}",
+                        snapshot.snapshot_id(),
+                        snapshot.sequence_number(),
+                        self.last_sequence_number
+                    ),
+                ));
+            }
+        }
+
         Ok(())
     }
 
@@ -2015,6 +2033,89 @@ mod tests {
             .contains("Snapshot for reference foo does not exist in the existing snapshots list"));
     }
 
+    #[test]
+    fn test_v2_wrong_max_snapshot_sequence_number() {
+        let data = r#"
+        {
+            "format-version": 2,
+            "table-uuid": "9c12d441-03fe-4693-9a96-a0705ddf69c1",
+            "location": "s3://bucket/test/location",
+            "last-sequence-number": 1,
+            "last-updated-ms": 1602638573590,
+            "last-column-id": 3,
+            "current-schema-id": 0,
+            "schemas": [
+                {
+                    "type": "struct",
+                    "schema-id": 0,
+                    "fields": [
+                        {
+                            "id": 1,
+                            "name": "x",
+                            "required": true,
+                            "type": "long"
+                        }
+                    ]
+                }
+            ],
+            "default-spec-id": 0,
+            "partition-specs": [
+                {
+                    "spec-id": 0,
+                    "fields": []
+                }
+            ],
+            "last-partition-id": 1000,
+            "default-sort-order-id": 0,
+            "sort-orders": [
+                {
+                    "order-id": 0,
+                    "fields": []
+                }
+            ],
+            "properties": {},
+            "current-snapshot-id": 3055729675574597004,
+            "snapshots": [
+                {
+                    "snapshot-id": 3055729675574597004,
+                    "timestamp-ms": 1555100955770,
+                    "sequence-number": 4,
+                    "summary": {
+                        "operation": "append"
+                    },
+                    "manifest-list": "s3://a/b/2.avro",
+                    "schema-id": 0
+                }
+            ],
+            "statistics": [],
+            "snapshot-log": [],
+            "metadata-log": []
+        }
+    "#;
+
+        let err = serde_json::from_str::<TableMetadata>(data).unwrap_err();
+        println!("{}", err);
+        assert!(err.to_string().contains(
+            "Invalid snapshot with id 3055729675574597004 and sequence number 4 greater than last sequence number 1"
+        ));
+
+        // Change max sequence number to 4 - should work
+        let data = data.replace(
+            r#""last-sequence-number": 1,"#,
+            r#""last-sequence-number": 4,"#,
+        );
+        let metadata = serde_json::from_str::<TableMetadata>(data.as_str()).unwrap();
+        assert_eq!(metadata.last_sequence_number, 4);
+
+        // Change max sequence number to 5 - should work
+        let data = data.replace(
+            r#""last-sequence-number": 4,"#,
+            r#""last-sequence-number": 5,"#,
+        );
+        let metadata = serde_json::from_str::<TableMetadata>(data.as_str()).unwrap();
+        assert_eq!(metadata.last_sequence_number, 5);
+    }
+
     #[test]
     fn test_statistic_files() {
         let data = r#"