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]