Skip to content

fix: dir namespace cloud storage path removes one subdir level#5464

Merged
jackye1995 merged 1 commit intolance-format:mainfrom
jackye1995:create-empty-location
Dec 12, 2025
Merged

fix: dir namespace cloud storage path removes one subdir level#5464
jackye1995 merged 1 commit intolance-format:mainfrom
jackye1995:create-empty-location

Conversation

@jackye1995
Copy link
Copy Markdown
Contributor

We use uri_to_url to convert a root location to Url and then add table subpath in dir namespace. This causes issue because there is no trailing slash for the uri, and Url::join would treat the last part of the path as a file, so joining it with another file would cause the last part of the sub directory to be ignored.

This PR ensures that we add a trailing slash to the url. uri_to_url is used all over the place to convert root paths, so far no place do join after the conversion except for dir namespace, so there is no impact on other places. But I think a more comprehensive fix would be to ensure a trailing slash in the function directly, but it will be a fix that affects more places so will do it later after examining all the use cases more carefully.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@github-actions github-actions Bot added the bug Something isn't working label Dec 12, 2025
@jackye1995 jackye1995 merged commit ba35dcb into lance-format:main Dec 12, 2025
27 checks passed
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 12, 2025

Codecov Report

❌ Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-namespace-impls/src/dir/manifest.rs 96.55% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

jackye1995 added a commit to lance-format/lance-spark that referenced this pull request Dec 12, 2025
This contains fix to directory namespace error with cloud storage path
lance-format/lance#5464
jackye1995 added a commit to jackye1995/lance that referenced this pull request Dec 16, 2025
…-format#5464)

We use `uri_to_url` to convert a root location to `Url` and then add
table subpath in dir namespace. This causes issue because there is no
trailing slash for the `uri`, and `Url::join` would treat the last part
of the path as a file, so joining it with another file would cause the
last part of the sub directory to be ignored.

This PR ensures that we add a trailing slash to the url. `uri_to_url` is
used all over the place to convert root paths, so far no place do `join`
after the conversion except for dir namespace, so there is no impact on
other places. But I think a more comprehensive fix would be to ensure a
trailing slash in the function directly, but it will be a fix that
affects more places so will do it later after examining all the use
cases more carefully.
jackye1995 added a commit to jackye1995/lance that referenced this pull request Jan 21, 2026
…-format#5464)

We use `uri_to_url` to convert a root location to `Url` and then add
table subpath in dir namespace. This causes issue because there is no
trailing slash for the `uri`, and `Url::join` would treat the last part
of the path as a file, so joining it with another file would cause the
last part of the sub directory to be ignored.

This PR ensures that we add a trailing slash to the url. `uri_to_url` is
used all over the place to convert root paths, so far no place do `join`
after the conversion except for dir namespace, so there is no impact on
other places. But I think a more comprehensive fix would be to ensure a
trailing slash in the function directly, but it will be a fix that
affects more places so will do it later after examining all the use
cases more carefully.
jiaoew1991 pushed a commit to jiaoew1991/lance-spark that referenced this pull request Feb 25, 2026
This contains fix to directory namespace error with cloud storage path
lance-format/lance#5464
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants