Xuanwo commented on code in PR #7815: URL: https://github.com/apache/opendal/pull/7815#discussion_r3464447357
########## core/core/src/docs/rfcs/7815_rename_if_not_exists.md: ########## Review Comment: RFC should live in a dedicated PR. ########## core/core/src/docs/rfcs/7815_rename_if_not_exists.md: ########## @@ -0,0 +1,199 @@ +- Proposal Name: `rename_if_not_exists` +- Start Date: 2026-06-23 +- RFC PR: [apache/opendal#7815](https://github.com/apache/opendal/pull/7815) +- Tracking Issue: [apache/opendal#0000](https://github.com/apache/opendal/issues) + +# Summary + +Add `Operator::rename_if_not_exists` and +`blocking::Operator::rename_if_not_exists`. + +The operation renames a source file only when the destination file does not +exist. If the destination file exists, it returns `AlreadyExists` and leaves +both files unchanged. The existing `rename` API keeps its overwrite semantics. + +# Motivation + +OpenDAL defines `rename` as an overwrite operation. This gives users consistent +behavior across services even when a backend's native rename does not overwrite +the destination. + +Some applications also need to publish a file only if no other writer has +already published the destination. Implementing this as `stat` followed by +`rename` is unsafe because another writer can create the destination between +the two operations. A service-specific configuration flag has a different +problem: it changes the meaning of the standard `rename` operation and makes +portable code depend on backend configuration. + +OpenDAL needs a separate operation that expresses the destination condition +directly and allows services to advertise whether they can enforce it. + +# Guide-level explanation + +Use `rename` when an existing destination should be replaced: + +```rust +op.rename("staging/file", "published/file").await?; +``` + +Use `rename_if_not_exists` when an existing destination must be preserved: + +```rust +use opendal::{ErrorKind, Operator, Result}; + +async fn publish(op: Operator) -> Result<()> { + match op + .rename_if_not_exists("staging/file", "published/file") + .await + { + Ok(()) => Ok(()), + Err(err) if err.kind() == ErrorKind::AlreadyExists => { + // Another writer has already published the destination. + Err(err) + } + Err(err) => Err(err), + } +} +``` + +The operation has the following outcomes: + +- If the destination does not exist, the source is renamed to the destination. +- If the destination file exists, the operation returns `AlreadyExists` and + leaves both source and destination unchanged. +- If the service cannot enforce this condition, the operation returns + `Unsupported`. +- If source and destination are the same path, the operation returns + `IsSameFile`, matching `rename`. + +Users can check `Capability::rename_with_if_not_exists` before calling the +operation. + +# Reference-level explanation + +## Public API + +Add asynchronous and blocking operator methods: + +```rust +impl Operator { + pub async fn rename_if_not_exists(&self, from: &str, to: &str) + -> Result<()>; +} + +impl blocking::Operator { + pub fn rename_if_not_exists(&self, from: &str, to: &str) -> Result<()>; +} +``` + +The normal `rename` method continues to overwrite an existing destination. The +new method is a separate API so callers select the behavior at each call site. + +## Raw API + +Extend `OpRename` with an `if_not_exists` argument: + +```rust +pub struct OpRename { + if_not_exists: bool, +} + +impl OpRename { + pub fn with_if_not_exists(self, if_not_exists: bool) -> Self; + pub fn if_not_exists(&self) -> bool; +} +``` + +The `Access::rename` signature does not change. Services inspect `OpRename` to +select native overwrite or no-overwrite behavior. + +Add a capability field: + +```rust +pub struct Capability { + pub rename: bool, + pub rename_with_if_not_exists: bool, +} +``` + +`rename_with_if_not_exists` is meaningful only when `rename` is also supported. +The correctness check layer returns `Unsupported` when the conditional argument +is requested but the service does not advertise the capability. + +## Atomicity + +A service must advertise `rename_with_if_not_exists` only when it can enforce +the destination condition as part of the rename. A separate destination check +followed by an overwriting rename does not satisfy the contract because it has +a time-of-check to time-of-use race. + +HDFS supports this through its native rename without the overwrite option. +The libhdfs `hdfsRename` call uses `Options.Rename.NONE`, so the destination is +not replaced if it appears concurrently. HDFS uses that operation directly for +`rename_if_not_exists`. For normal `rename`, the HDFS service removes an +existing destination first and then performs the native rename, preserving +OpenDAL's existing overwrite semantics. + +# Drawbacks + +The public operator and capability surfaces gain another method and field. +Service implementations must distinguish two rename modes and must not +advertise the conditional capability when they can only simulate it with a +racy preflight check. + +The API also exposes a semantic operation that some backends cannot support, so +portable callers must handle `Unsupported` or inspect the capability. + +# Rationale and alternatives Review Comment: Please check with other services like fs, s3 and check how rename works for them. ########## core/core/src/docs/rfcs/7815_rename_if_not_exists.md: ########## @@ -0,0 +1,199 @@ +- Proposal Name: `rename_if_not_exists` +- Start Date: 2026-06-23 +- RFC PR: [apache/opendal#7815](https://github.com/apache/opendal/pull/7815) +- Tracking Issue: [apache/opendal#0000](https://github.com/apache/opendal/issues) + +# Summary + +Add `Operator::rename_if_not_exists` and +`blocking::Operator::rename_if_not_exists`. + +The operation renames a source file only when the destination file does not +exist. If the destination file exists, it returns `AlreadyExists` and leaves +both files unchanged. The existing `rename` API keeps its overwrite semantics. + +# Motivation + +OpenDAL defines `rename` as an overwrite operation. This gives users consistent +behavior across services even when a backend's native rename does not overwrite +the destination. + +Some applications also need to publish a file only if no other writer has +already published the destination. Implementing this as `stat` followed by +`rename` is unsafe because another writer can create the destination between +the two operations. A service-specific configuration flag has a different +problem: it changes the meaning of the standard `rename` operation and makes +portable code depend on backend configuration. + +OpenDAL needs a separate operation that expresses the destination condition +directly and allows services to advertise whether they can enforce it. + +# Guide-level explanation + +Use `rename` when an existing destination should be replaced: + +```rust +op.rename("staging/file", "published/file").await?; +``` + +Use `rename_if_not_exists` when an existing destination must be preserved: + +```rust +use opendal::{ErrorKind, Operator, Result}; + +async fn publish(op: Operator) -> Result<()> { + match op + .rename_if_not_exists("staging/file", "published/file") + .await + { + Ok(()) => Ok(()), + Err(err) if err.kind() == ErrorKind::AlreadyExists => { + // Another writer has already published the destination. + Err(err) + } + Err(err) => Err(err), + } +} +``` + +The operation has the following outcomes: + +- If the destination does not exist, the source is renamed to the destination. +- If the destination file exists, the operation returns `AlreadyExists` and + leaves both source and destination unchanged. +- If the service cannot enforce this condition, the operation returns + `Unsupported`. +- If source and destination are the same path, the operation returns + `IsSameFile`, matching `rename`. + +Users can check `Capability::rename_with_if_not_exists` before calling the +operation. + +# Reference-level explanation + +## Public API + +Add asynchronous and blocking operator methods: + +```rust +impl Operator { + pub async fn rename_if_not_exists(&self, from: &str, to: &str) + -> Result<()>; +} + +impl blocking::Operator { + pub fn rename_if_not_exists(&self, from: &str, to: &str) -> Result<()>; +} +``` + +The normal `rename` method continues to overwrite an existing destination. The +new method is a separate API so callers select the behavior at each call site. + +## Raw API + +Extend `OpRename` with an `if_not_exists` argument: + +```rust +pub struct OpRename { + if_not_exists: bool, +} + +impl OpRename { + pub fn with_if_not_exists(self, if_not_exists: bool) -> Self; + pub fn if_not_exists(&self) -> bool; +} +``` + +The `Access::rename` signature does not change. Services inspect `OpRename` to Review Comment: We have renamed `Access` trait. ########## core/core/src/docs/rfcs/7815_rename_if_not_exists.md: ########## @@ -0,0 +1,199 @@ +- Proposal Name: `rename_if_not_exists` +- Start Date: 2026-06-23 +- RFC PR: [apache/opendal#7815](https://github.com/apache/opendal/pull/7815) +- Tracking Issue: [apache/opendal#0000](https://github.com/apache/opendal/issues) + +# Summary + +Add `Operator::rename_if_not_exists` and +`blocking::Operator::rename_if_not_exists`. + +The operation renames a source file only when the destination file does not +exist. If the destination file exists, it returns `AlreadyExists` and leaves +both files unchanged. The existing `rename` API keeps its overwrite semantics. + +# Motivation + +OpenDAL defines `rename` as an overwrite operation. This gives users consistent +behavior across services even when a backend's native rename does not overwrite +the destination. + +Some applications also need to publish a file only if no other writer has +already published the destination. Implementing this as `stat` followed by +`rename` is unsafe because another writer can create the destination between +the two operations. A service-specific configuration flag has a different +problem: it changes the meaning of the standard `rename` operation and makes +portable code depend on backend configuration. + +OpenDAL needs a separate operation that expresses the destination condition +directly and allows services to advertise whether they can enforce it. + +# Guide-level explanation + +Use `rename` when an existing destination should be replaced: + +```rust +op.rename("staging/file", "published/file").await?; +``` + +Use `rename_if_not_exists` when an existing destination must be preserved: + +```rust +use opendal::{ErrorKind, Operator, Result}; + +async fn publish(op: Operator) -> Result<()> { + match op + .rename_if_not_exists("staging/file", "published/file") Review Comment: We shouldn't provide such API, please check with how `copy_if_not_exists` works. -- 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]
