RFC: rename if not exists#7818
Conversation
dfdbf69 to
4575406
Compare
|
@Xuanwo @erickguan Hi, could you please help review this RFC PR when free? Thanks very much! |
erickguan
left a comment
There was a problem hiding this comment.
Good writing. I added some comments. I also have questions on atomicity.
You will also need Xuanwo's approval :)
|
|
||
| ### Multi-step overwrite services | ||
|
|
||
| `aliyun_drive`, `gdrive`, `goosefs`, and `koofr` currently delete or trash an |
There was a problem hiding this comment.
Depending on if we are changing rename to be atomic, say yes. Then we are essentially making these services' rename to be a simulation. Either this is way beyond the scope and we register this problem as a future work. Or we would see a way to address this problem in this RFC. e.g., a capability flag for declaring so? or documentation on atomicity with/without rename_with_if_not_exists.
There was a problem hiding this comment.
I think the answer is neither — this RFC does NOT propose making rename atomic. Multi-step delete+rename is a valid implementation of overwrite, not a "simulation." It genuinely fulfills the contract: the destination ends up with the source content.
The atomicity gate is introduced only for rename_with_if_not_exists because it adds a conditional check that would be meaningless without atomic enforcement. Overwrite rename (the default) has no such check, so no new atomicity requirement.
That said, a capability flag like rename_is_atomic for the base rename operation is a reasonable idea, but it is separate from this RFC. I would prefer keeping this RFC scoped to if_not_exists and filing a follow-up issue for rename atomicity documentation/capability if the maintainers agree it is worth tracking.
There was a problem hiding this comment.
@hfutatzhanghb Is zhb your handle on OpenDAL discord?
Co-authored-by: Erick Guan <297343+erickguan@users.noreply.github.com>
Co-authored-by: Erick Guan <297343+erickguan@users.noreply.github.com>
Co-authored-by: Erick Guan <297343+erickguan@users.noreply.github.com>
Co-authored-by: Erick Guan <297343+erickguan@users.noreply.github.com>
Address erickguan's review: explicitly document that RenameOptions is always constructable, matching the existing write and copy pattern where the correctness_check layer returns Unsupported when the capability is not advertised.
Replace ambiguous 'the destination condition' with explicit phrasing: 'destination-does-not-exist check' and 'destination already exists (if_not_exists condition fails)'. Addresses erickguan's review feedback.
Replace the ambiguous 'the operation' with 'the rename' for clarity. Addresses erickguan's review feedback.
Add an explicit paragraph explaining that overwrite rename does not require atomicity (multi-step delete+rename achieves the same result), while the if_not_exists condition must be checked atomically with the move. A backend with non-atomic overwrite may still support conditional rename by skipping the pre-delete and relying on the native conflict rejection. HDFS is the example. Addresses erickguan's review feedback about the apparent contradiction between non-atomic overwrite implementations and the atomicity requirement for conditional rename.
|
This RFC will require a tracking issue once approved :) |
|
Should i create an issue and update rfc now?
…---- Replied Message ----
| From | Erick ***@***.***> |
| Date | 06/24/2026 21:51 |
| To | ***@***.***> |
| Cc | ***@***.***>***@***.***> |
| Subject | Re: [apache/opendal] RFC: rename if not exists (PR #7818) |
erickguan left a comment (apache/opendal#7818)
This RFC will require a tracking issue once approved :)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
@hfutatzhanghb Up to you. I would suggest waiting Xuanwo's review. |
|
Got it!
…---- Replied Message ----
| From | Erick ***@***.***> |
| Date | 06/24/2026 22:05 |
| To | ***@***.***> |
| Cc | ***@***.***>***@***.***> |
| Subject | Re: [apache/opendal] RFC: rename if not exists (PR #7818) |
erickguan left a comment (apache/opendal#7818)
@hfutatzhanghb Up to you. I would suggest waiting Xuanwo's review.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Hi, @Xuanwo please cc when have free time. Thanks very much! |
Xuanwo
left a comment
There was a problem hiding this comment.
LGTM, thank you @hfutatzhanghb for working on this and thank you @erickguan for the review!
Which issue does this PR close?
N/A.
Rationale for this change
OpenDAL needs a documented cross-service contract for conditional rename before
extending the public API. The design must align with existing write and copy
options while preserving the atomic destination condition.
What changes are included in this PR?
rename_with(...).if_not_exists(true)andRenameOptions.AlreadyExistsandUnsupportedbehavior.Are there any user-facing changes?
No implementation is included in this RFC PR. If accepted, the proposed public
API and capability will be implemented separately.
AI Usage Statement
AI assistance (GPT-5.5 with Codex) was used to prepare this RFC.
Reviewed by GLM-5.2 with Codex