JingsongLi commented on code in PR #1023:
URL: https://github.com/apache/paimon-rust/pull/1023#discussion_r4176818981


##########
crates/paimon/src/table/tag_manager.rs:
##########
@@ -269,6 +269,33 @@ impl TagManager {
         self.file_io.delete_file(&path).await
     }
 
+    /// Rename a tag, preserving its snapshot pointer and the `tagCreateTime` /
+    /// `tagTimeRetained` metadata by moving the raw tag file. Errors if the
+    /// source tag is missing or the target name already exists.
+    ///
+    /// Uses copy-then-delete rather than a native rename: object stores and 
the
+    /// in-memory backend do not support rename.
+    pub async fn rename(&self, tag_name: &str, target_tag_name: &str) -> 
crate::Result<()> {
+        validate_tag_name(tag_name)?;
+        validate_tag_name(target_tag_name)?;
+        let src = self.tag_path(tag_name);
+        let dst = self.tag_path(target_tag_name);
+        if !self.file_io.new_input(&src)?.exists().await? {
+            return Err(crate::Error::DataInvalid {
+                message: format!("tag '{tag_name}' does not exist"),
+                source: None,
+            });
+        }
+        if self.file_io.new_input(&dst)?.exists().await? {
+            return Err(crate::Error::DataInvalid {
+                message: format!("tag '{target_tag_name}' already exists"),
+                source: None,
+            });
+        }
+        self.file_io.copy_file(&src, &dst).await?;

Review Comment:
   [P1] Preserve Java's no-overwrite behavior when renaming a tag
   
   `FileIO::copy_file` overwrites an existing destination, so the earlier 
`exists()` check does not protect concurrent renames. On a real filesystem, 
joining `rename(a, release)` and `rename(b, release)` where a pins snapshot 1 
and b pins snapshot 3 reproduced this on the first attempt: both returned 
`Ok(())`, both originals were deleted, and the remaining tag pinned only 
snapshot 1. This removes the other historical snapshot's retention/time-travel 
entry and can allow later expiry to reclaim its data. Java's 
[TagManager.renameTag](https://github.com/apache/paimon/blob/master/paimon-core/src/main/java/org/apache/paimon/utils/TagManager.java)
 delegates to 
[LocalFileIO.rename](https://github.com/apache/paimon/blob/master/paimon-common/src/main/java/org/apache/paimon/fs/local/LocalFileIO.java),
 which serializes the local move and refuses an existing destination. Please 
align with that behavior: publish without overwriting a concurrently created 
target, and only remove the source after s
 uccessful publication. The losing source tag must remain.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to