fix: Update view version log timestamp for historical accuracy (#1218)

## What changes are included in this PR?

This change updates the behavior when setting a view version as current.
When we set a previously existing view version as current, we now update
its log timestamp to the current time rather than reusing the original
timestamp of the ViewVersion.

Lets assume the following changes:
1. ViewVersion 1 is added and set active
2. View Version 2 is added and set active
3. View Version 1 is set active.

This resulted in the following `version_log`:
```json
    "version-log": [
        {
            "version-id": 1,
            "timestamp-ms": 1744716416168
        },
        {
            "version-id": 2,
            "timestamp-ms": 1744716449754
        },
        {
            "version-id": 1,
            "timestamp-ms": 1744716416168
        }
    ],
```

Note that the last and first entries have the same timestamp, namely the
time stamp of the initial ViewVersion creation.

I believe this is undesired in a history, where we are interested when a
certain change became active. It makes sense to use the exact timestamp
of the ViewVersion if it was added in the same set of changes, but
re-enabling a previously used view version (maybe years ago) should not
add a history for this past timestamp.

Java behaviors the same way as rust currently does. @Fokko @nastra if we
agree that we should change the behavior, we should also touch Java.

## Are these changes tested?

Yes

Author: c-thiel

crates/iceberg/src/spec/view_metadata.rs

@@ -229,6 +229,12 @@ impl ViewVersionLog {
     pub fn timestamp(&self) -> Result<DateTime<Utc>> {
         timestamp_ms_to_utc(self.timestamp_ms)
     }
+
+    /// Update the timestamp of this version log.
+    pub(crate) fn set_timestamp_ms(&mut self, timestamp_ms: i64) -> &mut Self {
+        self.timestamp_ms = timestamp_ms;
+        self
+    }
 }
 
 pub(super) mod _serde {

crates/iceberg/src/spec/view_metadata_builder.rs

@@ -219,7 +219,20 @@ impl ViewMetadataBuilder {
             });
         }
 
-        self.history_entry = Some(version.log());
+        // Use the timestamp of the snapshot if it was added in this set of changes,
+        // otherwise use a current timestamp for the log. The view version was added
+        // by a past transaction.
+        let version_added_in_this_changes = self
+            .changes
+            .iter()
+            .any(|update| matches!(update, ViewUpdate::AddViewVersion { view_version } if view_version.version_id() == version_id));
+
+        let mut log = version.log();
+        if !version_added_in_this_changes {
+            log.set_timestamp_ms(Utc::now().timestamp_millis());
+        }
+
+        self.history_entry = Some(log);
 
         Ok(self)
     }
@@ -257,9 +270,8 @@ impl ViewMetadataBuilder {
             // in this case. I prefer to add changes as the state of the builder is
             // potentially mutated (`last_added_version_id`), thus we should record the change.
             if self.last_added_version_id != Some(version_id) {
-                self.changes.push(ViewUpdate::AddViewVersion {
-                    view_version: view_version.with_version_id(version_id),
-                });
+                self.changes
+                    .push(ViewUpdate::AddViewVersion { view_version });
                 self.last_added_version_id = Some(version_id);
             }
             return Ok(version_id);
@@ -293,7 +305,6 @@ impl ViewMetadataBuilder {
 
         require_unique_dialects(&view_version)?;
 
-        // ToDo Discuss: This check is not present in Java.
         // The `TableMetadataBuilder` uses these checks in multiple places - also in Java.
         // If we think delayed requests are a problem, I think we should also add it here.
         if let Some(last) = self.metadata.version_log.last() {
@@ -852,6 +863,54 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_use_previously_added_version() {
+        let v2 = new_view_version(2, 1, "select 1 as count");
+        let v3 = new_view_version(3, 1, "select count(1) as count from t2");
+        let schema = Schema::builder().build().unwrap();
+
+        let log_v2 = ViewVersionLog::new(2, v2.timestamp_ms());
+        let log_v3 = ViewVersionLog::new(3, v3.timestamp_ms());
+
+        let metadata_v2 = builder_without_changes()
+            .set_current_version(v2.clone(), schema.clone())
+            .unwrap()
+            .build()
+            .unwrap()
+            .metadata;
+
+        // Log should use the exact timestamp of v1
+        assert_eq!(metadata_v2.version_log.last().unwrap(), &log_v2);
+
+        // Add second version, should use exact timestamp of v2
+        let metadata_v3 = metadata_v2
+            .into_builder()
+            .set_current_version(v3.clone(), schema)
+            .unwrap()
+            .build()
+            .unwrap()
+            .metadata;
+
+        assert_eq!(metadata_v3.version_log[1..], vec![
+            log_v2.clone(),
+            log_v3.clone()
+        ]);
+
+        // Re-use Version 1, add a new log entry with a new timestamp
+        let metadata_v4 = metadata_v3
+            .into_builder()
+            .set_current_version_id(2)
+            .unwrap()
+            .build()
+            .unwrap()
+            .metadata;
+
+        // Last entry should be equal to v2 but with an updated timestamp
+        let entry = metadata_v4.version_log.last().unwrap();
+        assert_eq!(entry.version_id(), 2);
+        assert!(entry.timestamp_ms() > v2.timestamp_ms());
+    }
+
     #[test]
     fn test_assign_uuid() {
         let builder = builder_without_changes();