Read hf:// URLs from DuckDB by mounting stores where the registry says - #9265
Merged
Conversation
…try says DuckDB resolved scan URLs by clearing the URL path and mounting the store at the authority, then globbing the full URL path against it. That assumption held for every scheme so far, but not for `hf://`: a Hugging Face store is rooted at a repository and revision, which occupy path segments, so the authority-rooted URL (`hf://datasets`) does not even name a repository and the full path would address the wrong keys. Resolve the full URL instead and glob the path the registry reports — for authority-mounted schemes that is the whole URL path, so their behavior is unchanged, and only the registry knows how deep any other scheme mounts. The per-bind filesystem cache goes away because the registry already caches one client per store prefix; the filesystem wrapper rebuilt per glob is a thin adapter. This also percent-decodes the globbed path, which the raw URL path was not. The Hub serves no listing, so wildcard globs over `hf://` fail with the store's listing error; exact file paths work because a glob-free pattern is resolved with `head` alone. The Java binding still cannot read `hf://`: it keys reads by the full URL path against a store built by its own scheme dispatch, so it needs the same move to registry-reported mounting. Left for a follow-up. Towards #5379. Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lc5zw7Le2T3pakDEUdKTYd
myrrc
approved these changes
Aug 7, 2026
myrrc
left a comment
Contributor
There was a problem hiding this comment.
LGTM but sometimes in the future we should use duckdb's IO so that vortex library wouldn't bundle all these libraries
Contributor
Author
|
yes, this is really just unifying the logic everywhere |
…al module `crate::opendal` only exists under a service feature (cos/oss/goosefs), so the intra-doc link in the hf module header dangles in an hf-only build. The `supports_scheme` doc had the same problem and was un-linked in review; this is the one that was missed. Verified with `cargo doc` under `hf,registry` alone and under `--all-features`. Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lc5zw7Le2T3pakDEUdKTYd
robert3005
marked this pull request as ready for review
August 7, 2026 11:20
robert3005
enabled auto-merge (squash)
August 7, 2026 12:46
AdamGS
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Duckdb filesystem resolution uses default registry behaviour