You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "yuhao-su (via GitHub)" <gi...@apache.org> on 2026/09/30 19:11:22 UTC
[PR] fix(storage-azdls): drop the leading slash from relative paths [iceberg-rust]
yuhao-su opened a new pull request, #3311:
URL: https://github.com/apache/iceberg-rust/pull/3311
## Which issue does this PR close?
- None. Found while testing against a real ADLS Gen2 account.
## What changes are included in this PR?
ADLS relative paths kept the leading `/` of the URL path (`/dir/file`), unlike the other backends. OpenDAL's `Operator` methods strip it, but its `Deleter` does not, so `FileIO::delete_stream` sends `DELETE <endpoint>/<filesystem>//dir/file` and ADLS answers 400 `InvalidUri`; debug builds panic on OpenDAL's `debug_assert!(!path.starts_with('/'))` instead. `drop_table_data`, used by `purge_table` in the memory, SQL, Glue, and HMS catalogs, therefore fails on ADLS.
`AzureStoragePath::relative_path` now returns the path without the leading `/`, and both `azdls_create_operator` and `relativize_path` use it.
Related:
- apache/opendal#8352 makes OpenDAL's `Deleter` normalize paths. This change does not depend on it.
- #3163 changes how `AzureStoragePath::path` is derived and adds test cases that expect the leading `/`, so whichever lands second needs a small update.
## Are these changes tested?
Unit tests: updated the relative-path expectations in `test_azdls_create_operator` and `test_relativize_path_azdls`, and added a filesystem-root case. There is no ADLS integration test in this repo. The same change was verified in a downstream fork against a real ADLS Gen2 account: `delete_stream` returned 400 `InvalidUri` before it and succeeded after it.
## AI Disclosure
Written with AI assistance (Claude Code). I reviewed the change and the test results above.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: [PR] fix(storage-azdls): drop the leading slash from relative paths [iceberg-rust]
Posted by "CTTY (via GitHub)" <gi...@apache.org>.
CTTY merged PR #3311:
URL: https://github.com/apache/iceberg-rust/pull/3311
--
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: [PR] fix(storage-azdls): drop the leading slash from relative paths [iceberg-rust]
Posted by "yuhao-su (via GitHub)" <gi...@apache.org>.
yuhao-su commented on code in PR #3311:
URL: https://github.com/apache/iceberg-rust/pull/3311#discussion_r4148986397
##########
crates/storage/opendal/src/azdls.rs:
##########
@@ -237,6 +236,13 @@ impl AzureStoragePath {
self.endpoint_suffix
)
}
+
+ /// The part of `absolute_path` below the filesystem, without a leading `/`,
+ /// like the relative paths of the other backends.
+ pub(crate) fn relative_path<'a>(&self, absolute_path: &'a str) -> &'a str {
+ let path = &absolute_path[absolute_path.len() - self.path.len()..];
+ path.strip_prefix('/').unwrap_or(path)
Review Comment:
The slicing is unchanged from `main` (`split_at` in `azdls_create_operator`, `&path[path.len() - relative_path_len..]` in `relativize_path`); this PR only moves it into a helper. The mismatch comes from `AzureStoragePath::path` being `Url::path()`, which is percent-encoded and normalized, so it is not a byte suffix of the input for paths with non-ASCII characters or `#`. #3163 fixes that at the source by taking the raw suffix of the input, so I'd rather leave this to #3163 than add a second fix here.
--
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: [PR] fix(storage-azdls): drop the leading slash from relative paths [iceberg-rust]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3311:
URL: https://github.com/apache/iceberg-rust/pull/3311#discussion_r4148642978
##########
crates/storage/opendal/src/azdls.rs:
##########
@@ -237,6 +236,13 @@ impl AzureStoragePath {
self.endpoint_suffix
)
}
+
+ /// The part of `absolute_path` below the filesystem, without a leading `/`,
+ /// like the relative paths of the other backends.
+ pub(crate) fn relative_path<'a>(&self, absolute_path: &'a str) -> &'a str {
+ let path = &absolute_path[absolute_path.len() - self.path.len()..];
+ path.strip_prefix('/').unwrap_or(path)
Review Comment:
This performs unchecked byte slicing into `absolute_path` based on `self.path.len()`, which will panic if the provided `absolute_path` doesn't exactly correspond to `self.path`'s length (e.g., normalization differences or accidental misuse of the API). Since this is a reusable helper, it should defensively validate assumptions. Consider (mandatory) changing this helper to avoid panics by (a) ensuring `absolute_path.ends_with(&self.path)` and deriving the slice via `strip_suffix`, or (b) using `get(start..)`, and returning a `Result<&str>` with an explicit error if the expected suffix isn't present. This makes the invariant explicit and prevents potential production panics.
--
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: [PR] fix(storage-azdls): drop the leading slash from relative paths [iceberg-rust]
Posted by "yuhao-su (via GitHub)" <gi...@apache.org>.
yuhao-su commented on PR #3311:
URL: https://github.com/apache/iceberg-rust/pull/3311#issuecomment-5918131781
@Xuanwo @CTTY Can you help take a look at this? It's a bug found when developing a Risingwave ADLS feature. Thank you!
--
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