You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "c-thiel (via GitHub)" <gi...@apache.org> on 2025/03/20 17:43:54 UTC
[I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
c-thiel opened a new issue, #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118
### Apache Iceberg Rust version
None
### Describe the bug
Currently when using the S3 FileIO's `remove_all` method we noticed the following behavior:
Running `remove_all('s3://foo/bar')` removes `s3://foo/bar*` so also siblings such as `s3://foo/bar-baz`.
I would expect fileio to understand that I provide a folder, and only this folder and subfolders are removed, not siblings that have the same prefix.
@Xuanwo, @liurenjie1024, @Fokko do you agree? If so should we fix it in iceberg-rust or opendal?
### To Reproduce
_No response_
### Expected behavior
`s3://foo/bar/*` is deleted, not `s3://foo/bar*`
### Willingness to contribute
I can contribute a fix for this bug independently
--
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: issues-unsubscribe@iceberg.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "c-thiel (via GitHub)" <gi...@apache.org>.
c-thiel commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2758499873
@Xuanwo I would slightly tend to only remove "abc" in your example, because its the safe default.
My main point is however that behavior should be consistent across storages.
GCS works like Azure. So out of those three, S3 is the odd one out.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2817810258
cc @Xuanwo Are we going to fix this ? It's the last issue in 0.5.0 release
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "c-thiel (via GitHub)" <gi...@apache.org>.
c-thiel commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2758504433
I think if a client calls `remove_all(table.location)`, this should work and be safe. So I would argue we should treat the input always as a directory / append a path.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2817896885
> cc [@Xuanwo](https://github.com/Xuanwo) Are we going to fix this ? It's the last issue in 0.5.0 release
Yep, let me do this in this week.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2782690687
Nice, let me handle this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2760648037
> I think if a client calls `remove_all(table.location)`, this should work and be safe. So I would argue we should treat the input always as a directory / append a path.
I have a suggestion that rename this API to `remove_dir_all` which indicate that it only removes a dir recursively. And in this API, we make sure that both `abc` and `abc/` works correctly. What do you think?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2837692421
> cc @Xuanwo Do you have time to work on this? I can handle it if you are busy.
Will finish this today 😚
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2766198351
> > I think if a client calls `remove_all(table.location)`, this should work and be safe. So I would argue we should treat the input always as a directory / append a path.
>
> I have a suggestion that rename this API to `remove_dir_all` which indicate that it only removes a dir recursively. And in this API, we make sure that both `abc` and `abc/` works correctly. What do you think?
>
> This also aligns with std's [`remove_dir_all`](https://doc.rust-lang.org/stable/std/fs/fn.remove_dir_all.html).
>
> Or are you expecting us to first check if `abc/` exists and then decide whether to remove a prefix or a directory?
>
> I'm fine with both approaches at the iceberg FileIO side.
+1 for following std's naming convention to have different methods for `remove_dir_all` and `remove_dir`(for empty dir)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2742082990
I guess this is opendal's behavior, and it seems reasonable to append "/" at end if not present. cc @Xuanwo
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2837667220
cc @Xuanwo Do you have time to work on this? I can handle it if you are busy.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "c-thiel (via GitHub)" <gi...@apache.org>.
c-thiel commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2741237615
Azure Filesystem behaves differently. Even without a trailing slash siblings are not deleted.
I think we should at least have consistent behavior, I would opt for the one described above.
The question remains what behavior is desired in opendal
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2742088402
OpenDAL treats input path as a prefix so remove_all("abc") will remove all keys that starts "abc" so it's expected that "abcd" will also be removed.
The ADLS Gen2 behavior is not expected to me, it might be a bug that need to address.
Back to iceberg, if we want to ensure that remove_all is just dir, we can always append a / to the path.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2760632715
Thank you @c-thiel, I will get this fixed.
And also, I confirmed that the opendal behavior in `ADLS Gen2` is wrong. I will take another look.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Unexpected FileIO `remove_all` behavior for S3 [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #1118:
URL: https://github.com/apache/iceberg-rust/issues/1118#issuecomment-2747157994
Hi, @c-thiel, what’s your expected behavior? I’m happy to help resolve this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org