feat(core): add conditional rename option by hfutatzhanghb · Pull Request #7815 · apache/opendal

hfutatzhanghb · 2026-06-23T03:13:39Z

Which issue does this PR close?

N/A.

Rationale for this change

OpenDAL's default rename contract overwrites an existing destination. Some
callers need an atomic publish operation that moves a source only when the
destination does not exist. A separate stat followed by rename cannot
provide this guarantee because another writer may create the destination
between the two operations.

This implementation follows the options-based API accepted in
RFC #7818.

What changes are included in this PR?

Add RenameOptions, Operator::rename_with(...).if_not_exists(true), and
blocking rename_options.
Add raw OpRename::if_not_exists and
Capability::rename_with_if_not_exists.
Reject conditional rename with Unsupported when a service does not
advertise the capability.
Enable the capability for HDFS using its native no-overwrite rename
operation.
Map an existing HDFS destination to ConditionNotMatch, including conflicts
reported by the final native rename, while leaving both paths unchanged.
Preserve the existing overwrite behavior of normal HDFS rename.
Add correctness checks, behavior tests, HDFS error-mapping coverage, and
user-facing documentation.

Are there any user-facing changes?

Yes. Rust users can request an atomic no-replace rename with:

op.rename_with("source", "destination")
    .if_not_exists(true)
    .await?;

Normal rename behavior remains unchanged. HDFS initially advertises the new
capability; other services return Unsupported unless they implement and
advertise the same atomic guarantee.

AI Usage Statement

OpenAI Codex (GPT-5) was used to prepare and review this PR.

hfutatzhanghb · 2026-06-23T03:49:36Z

@Xuanwo @erickguan Hi, could you please reivew this PR when have free time? Thanks!

erickguan

No objection to add a toggle to a service. But a comment on your function design.

minor (non-blocking): perhaps we could have better names or options to confirm delete target objects in the long run.

erickguan · 2026-06-23T05:24:13Z

+}
+
+#[allow(deprecated)]
+impl Default for HdfsConfig {


minor(non-blocking): we probably don't need Default in any of services.

@erickguan Thanks very much for reviewing. nice advice, have fixed and pushed.

erickguan · 2026-06-23T05:27:22Z

 }

+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum HdfsRenameTargetAction {


I would see either we have an enum to raise error on the callsite of hdfs_rename_existing_file_action. Otherwise, is returning Ok() enough?

have fixed.

Xuanwo · 2026-06-23T09:50:48Z

    pub enable_append: bool,
    /// atomic_write_dir of this backend
    pub atomic_write_dir: Option<String>,
+    /// Whether HDFS rename should overwrite an existing target file.


I think this feature is focusing on the wrong thing. The semantics of OpenDAL are clear: rename should overwrite, just like the file system does. Therefore, HDFS should not alter the behavior itself. Instead, it should:

Enable the atomic rename if HDFS supports it

Fall back to delete-then-rename if HDFS does not support it

Actually, HDFS supports atomic rename and suggests to use Options.Rename to perform rename operations which are defined as below:
https://github.com/apache/hadoop/blob/10bb684bb4d5cb8edbd2ce79a5d695592cef32c5/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Options.java#L226

IIUC, should i add a new operation interface atomic_rename_if_not_exists here and use it in lance?

I see. I believe the correct thing we should do here is to add a rename_if_not_exists for opendal.

hfutatzhanghb · 2026-06-23T13:18:21Z

@erickguan @Xuanwo Hi, have pushed latest changes. Could you please help review it when have free time? Thanks !

erickguan · 2026-06-23T15:55:20Z

If we want to extend an API, we would benefit from a small RFC for future reference. Could you build one RFC too?

hfutatzhanghb · 2026-06-24T01:40:24Z

If we want to extend an API, we would benefit from a small RFC for future reference. Could you build one RFC too?

Very nice advice. Have added a RFC. BTW. I think we can add a rule in AGENTS.md to constrain it. What's your opinions？

hfutatzhanghb · 2026-06-24T03:12:30Z

@erickguan @Xuanwo All checks passed. please cc. Thanks!

Xuanwo · 2026-06-24T04:02:26Z

RFC should live in a dedicated PR.

Xuanwo · 2026-06-24T04:03:27Z

+
+async fn publish(op: Operator) -> Result<()> {
+    match op
+        .rename_if_not_exists("staging/file", "published/file")


We shouldn't provide such API, please check with how copy_if_not_exists works.

Xuanwo · 2026-06-24T04:03:54Z

+}
+```
+
+The `Access::rename` signature does not change. Services inspect `OpRename` to


We have renamed Access trait.

Xuanwo · 2026-06-24T04:04:30Z

+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


Please check with other services like fs, s3 and check how rename works for them.

hfutatzhanghb · 2026-06-26T05:55:59Z

@Xuanwo @erickguan Hi, could you help review this PR when have free time? Thanks very much!

erickguan

Generally good. Some minor comments.

I don't understand why do we need a tuple for options and destination. Can you explain?

erickguan · 2026-06-26T15:12:44Z

+    /// use opendal_core::options::RenameOptions;
+    /// use opendal_core::Result;
+    ///
+    /// fn test(op: blocking::Operator) -> Result<()> {


Suggested change

/// fn test(op: blocking::Operator) -> Result<()> {

/// fn rename_with_options(op: blocking::Operator) -> Result<()> {

erickguan · 2026-06-26T15:18:14Z

+    /// use opendal_core::Operator;
+    /// use opendal_core::Result;
+    ///
+    /// async fn test(op: Operator) -> Result<()> {


Suggested change

/// async fn test(op: Operator) -> Result<()> {

/// async fn rename_with_options(op: Operator) -> Result<()> {

erickguan · 2026-06-26T15:20:32Z

+        ctx: OperationContext,
+        srv: Servicer,
+        from: String,
+        (opts, to): (options::RenameOptions, String),


Eh, what is so special with options and destination? Do we need a tuple?

erickguan · 2026-06-26T15:24:53Z

+            ErrorKind::ConditionNotMatch,
+            "target path already exists while if_not_exists is set",
+        )
+        .with_context("input", to_path)


Is input too vague here?

erickguan · 2026-06-26T15:26:33Z


 Refer to [`HdfsBuilder`]'s public API docs for more information.

+### Rename Behavior


We don't need documentation on this. Users will refer to operator's documentation.

erickguan · 2026-06-26T15:27:53Z

+
+/// Rename to a nonexistent path should succeed when if_not_exists is set.
+pub async fn test_rename_with_if_not_exists(op: Operator) -> Result<()> {
+    let parent = format!("{}/", uuid::Uuid::new_v4());


No need for parent since we have uuid here.

erickguan · 2026-06-26T15:29:01Z

+
+    op.write(&source_path, source_content.clone()).await?;
+
+    let target_path = format!("{parent}target");


What is the behavior to rename files into a destination without destination's parent folder? It might be tested but I didn't check.

erickguan · 2026-06-26T15:29:17Z

+pub async fn test_rename_with_if_not_exists_returns_condition_not_match(
+    op: Operator,
+) -> Result<()> {
+    let parent = format!("{}/", uuid::Uuid::new_v4());


No need to have parent.

hfutatzhanghb marked this pull request as ready for review June 23, 2026 03:14

hfutatzhanghb requested a review from Xuanwo as a code owner June 23, 2026 03:14

dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels Jun 23, 2026

hfutatzhanghb requested a review from tisonkun as a code owner June 23, 2026 03:31

erickguan requested changes Jun 23, 2026

View reviewed changes

dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jun 23, 2026

hfutatzhanghb requested a review from erickguan June 23, 2026 05:46

erickguan approved these changes Jun 23, 2026

View reviewed changes

dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 23, 2026

Xuanwo requested changes Jun 23, 2026

View reviewed changes

dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 23, 2026

hfutatzhanghb changed the title ~~feat(services/hdfs): add rename overwrite config~~ feat(core): add rename if not exists Jun 23, 2026

erickguan self-requested a review June 23, 2026 11:28

hfutatzhanghb force-pushed the codex/hdfs-rename-overwrite-config branch from 3be5d77 to c518749 Compare June 23, 2026 12:43

hfutatzhanghb marked this pull request as draft June 24, 2026 02:46

hfutatzhanghb marked this pull request as ready for review June 24, 2026 02:46

Xuanwo reviewed Jun 24, 2026

View reviewed changes

hfutatzhanghb changed the title ~~feat(core): add rename if not exists~~ feat(core): add conditional rename option Jun 24, 2026

hfutatzhanghb requested review from Zheaoli and messense as code owners June 24, 2026 07:51

hfutatzhanghb force-pushed the codex/hdfs-rename-overwrite-config branch from 1d58950 to 77ed302 Compare June 25, 2026 08:18

feat(core): support conditional rename

0a9a84d

hfutatzhanghb force-pushed the codex/hdfs-rename-overwrite-config branch from b3e66e3 to 0a9a84d Compare June 25, 2026 08:37

hfutatzhanghb requested a review from Xuanwo June 26, 2026 01:57

erickguan reviewed Jun 26, 2026

View reviewed changes

	/// fn test(op: blocking::Operator) -> Result<()> {
	/// fn rename_with_options(op: blocking::Operator) -> Result<()> {

	/// async fn test(op: Operator) -> Result<()> {
	/// async fn rename_with_options(op: Operator) -> Result<()> {


		Refer to [`HdfsBuilder`]'s public API docs for more information.

		### Rename Behavior


		op.write(&source_path, source_content.clone()).await?;

		let target_path = format!("{parent}target");

Uh oh!

Conversation

hfutatzhanghb commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

AI Usage Statement

Uh oh!

hfutatzhanghb commented Jun 23, 2026

Uh oh!

erickguan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hfutatzhanghb Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hfutatzhanghb commented Jun 23, 2026

Uh oh!

erickguan commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hfutatzhanghb commented Jun 24, 2026

Uh oh!

hfutatzhanghb commented Jun 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hfutatzhanghb commented Jun 26, 2026

Uh oh!

erickguan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hfutatzhanghb commented Jun 23, 2026 •

edited

Loading

hfutatzhanghb Jun 23, 2026 •

edited

Loading

erickguan commented Jun 23, 2026 •

edited

Loading