zeroshade commented on code in PR #1300:
URL: https://github.com/apache/iceberg-go/pull/1300#discussion_r3469876638


##########
io/local.go:
##########
@@ -80,3 +80,16 @@ func (LocalFS) Stat(name string) (fs.FileInfo, error) {
 func (LocalFS) Rename(oldpath, newpath string) error {
        return os.Rename(strings.TrimPrefix(oldpath, "file://"), 
strings.TrimPrefix(newpath, "file://"))
 }
+
+func (LocalFS) RenameNoReplace(oldpath, newpath string) error {
+       oldpath = strings.TrimPrefix(oldpath, "file://")
+       newpath = strings.TrimPrefix(newpath, "file://")
+
+       if err := os.Link(oldpath, newpath); err != nil {

Review Comment:
   `os.Link` fails with `EXDEV` across filesystems. Here temp and final share 
the metadata dir (same FS), so it is fine in practice, but a one-line doc note 
on the method would help since this is a new exported `io` primitive. On Linux, 
`unix.Renameat2(..., RENAME_NOREPLACE)` is the truly-atomic kernel primitive; 
the link-based approach is the conventional portable fallback. Reasonable as-is.



##########
io/local.go:
##########
@@ -80,3 +80,16 @@ func (LocalFS) Stat(name string) (fs.FileInfo, error) {
 func (LocalFS) Rename(oldpath, newpath string) error {
        return os.Rename(strings.TrimPrefix(oldpath, "file://"), 
strings.TrimPrefix(newpath, "file://"))
 }
+
+func (LocalFS) RenameNoReplace(oldpath, newpath string) error {
+       oldpath = strings.TrimPrefix(oldpath, "file://")
+       newpath = strings.TrimPrefix(newpath, "file://")
+
+       if err := os.Link(oldpath, newpath); err != nil {
+               return err
+       }
+
+       _ = os.Remove(oldpath)

Review Comment:
   Dropped `os.Remove` error: the publish already succeeded via `os.Link`, so 
this only risks leaking a stray temp file if removal fails (correctness is 
unaffected). A short comment noting it is intentional best-effort cleanup would 
prevent a future "unchecked error" flag.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to