(iceberg-rust) branch main updated: fix(views): make timestamp take &self & fix set_current_version_id (#1101)

[xu...@apache.org]

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 565839ed fix(views): make timestamp take &self & fix set_current_version_id (#1101)
565839ed is described below

commit 565839ed339c430acf25233c1f08c0cf60224de3
Author: Tobias Pütz <to...@hansetag.com>
AuthorDate: Tue Mar 18 10:27:12 2025 +0100

    fix(views): make timestamp take &self & fix set_current_version_id (#1101)
    
    ## Which issue does this PR close?
    
    <!--
    We generally require a GitHub issue to be filed for all bug fixes and
    enhancements and this helps us generate change logs for our releases.
    You can link an issue to this PR using the GitHub syntax. For example
    `Closes #123` indicates that this PR will close issue #123.
    -->
    
    - Closes #1099
    - Closes #1100
    
    ## What changes are included in this PR?
    
    1. fixes set current view version
    2. makes ViewVersionLog::timestamp not consume `self`
    
    ## Are these changes tested?
    
    1. Added a test which sets current view version to -1
    2. doesn't require a test, it's a quality of life improvement
    
    <!--
    Specify what test covers (unit test, integration test, etc.).
    
    If tests are not included in your PR, please explain why (for example,
    are they covered by existing tests)?
    -->
---
 crates/iceberg/src/spec/view_metadata.rs         |  2 +-
 crates/iceberg/src/spec/view_metadata_builder.rs | 30 ++++++++++++++++++++----
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/crates/iceberg/src/spec/view_metadata.rs b/crates/iceberg/src/spec/view_metadata.rs
index 162b6f4c..4c2be689 100644
--- a/crates/iceberg/src/spec/view_metadata.rs
+++ b/crates/iceberg/src/spec/view_metadata.rs
@@ -226,7 +226,7 @@ impl ViewVersionLog {
     }
 
     /// Returns the last updated timestamp as a DateTime<Utc> with millisecond precision.
-    pub fn timestamp(self) -> Result<DateTime<Utc>> {
+    pub fn timestamp(&self) -> Result<DateTime<Utc>> {
         timestamp_ms_to_utc(self.timestamp_ms)
     }
 }
diff --git a/crates/iceberg/src/spec/view_metadata_builder.rs b/crates/iceberg/src/spec/view_metadata_builder.rs
index 43b78c92..a07cdec5 100644
--- a/crates/iceberg/src/spec/view_metadata_builder.rs
+++ b/crates/iceberg/src/spec/view_metadata_builder.rs
@@ -181,14 +181,16 @@ impl ViewMetadataBuilder {
     /// - The specified `version_id` does not exist.
     /// - The specified `version_id` is `-1` but no version has been added.
     pub fn set_current_version_id(mut self, mut version_id: i32) -> Result<Self> {
-        if version_id == Self::LAST_ADDED && self.last_added_version_id.is_none() {
-            version_id = self.last_added_version_id.ok_or_else(|| {
-                Error::new(
+        if version_id == Self::LAST_ADDED {
+            let Some(last_added_id) = self.last_added_version_id else {
+                return Err(Error::new(
                     ErrorKind::DataInvalid,
                     "Cannot set current version id to last added version: no version has been added.",
-                )
-            })?;
+                ));
+            };
+            version_id = last_added_id;
         }
+
         let version_id = version_id; // make immutable
 
         if version_id == self.metadata.current_version_id {
@@ -1196,6 +1198,24 @@ mod test {
             .contains("Cannot set current version to unknown version with id: 10"));
     }
 
+    #[test]
+    fn test_set_current_version_to_last_added() {
+        let builder = builder_without_changes();
+        let v1 = new_view_version(2, 1, "select * from ns.tbl");
+        let v2 = new_view_version(3, 1, "select a,b from ns.tbl");
+        let meta = builder
+            .clone()
+            .add_version(v1)
+            .unwrap()
+            .add_version(v2)
+            .unwrap()
+            .set_current_version_id(-1)
+            .unwrap()
+            .build()
+            .unwrap();
+        assert_eq!(meta.metadata.current_version_id, 3);
+    }
+
     #[test]
     fn test_error_when_setting_negative_version_history_size() {
         let builder = builder_without_changes();