mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-25 13:12:39 +00:00
fix: support remote tables in storage root (#3881)
## Summary - allow `db://` to select the remote storage root when a host override supplies the endpoint - preserve the empty `x-lancedb-database` header used by the legacy clients for root storage - reject root-storage connections without a host override before constructing an invalid Cloud URL - document the supported URI and cover both accepted and rejected configurations ## Root cause The shared Rust remote URL parser required every `db://` URI to contain a host. That validation was introduced when the Node and Python clients moved to the Rust implementation, but the legacy Node client intentionally treated the empty host in `db://` as the storage-root selector. ## Validation - `cargo fmt --all` - `cargo test --quiet --features remote -p lancedb test_connect_remote_storage_root` - `cargo check --quiet --features remote --tests --examples` - `cargo clippy --quiet --features remote --tests --examples` - `cargo test --quiet --features remote --tests` Fixes #1944 <!-- lance-gatekeeper-fix:v1 agent=c71ca188791ad36fe5644c12960f758c generation=1 --> --------- Co-authored-by: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Co-authored-by: Xuanwo <github@xuanwo.io>
This commit is contained in:
co-authored by
Xuanwo
parent
dd4795028b
commit
6777a40dc2
@@ -1270,6 +1270,7 @@ pub struct ConnectRequest {
|
||||
/// - `/path/to/database` - local database on file system.
|
||||
/// - `s3://bucket/path/to/database` or `gs://bucket/path/to/database` - database on cloud object store
|
||||
/// - `db://dbname` - LanceDB Cloud
|
||||
/// - `db://` with a host override - remote tables in the storage root
|
||||
pub uri: String,
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
@@ -1382,6 +1383,8 @@ impl ConnectBuilder {
|
||||
///
|
||||
/// This option is only used when connecting to LanceDB Cloud (db:// URIs)
|
||||
/// and will be ignored for other URIs.
|
||||
/// Use the URI `db://` together with a host override to connect to remote
|
||||
/// tables stored directly in the storage root.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
@@ -2007,6 +2010,37 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
#[tokio::test]
|
||||
async fn test_connect_remote_storage_root() {
|
||||
let conn = ConnectBuilder::new("db://")
|
||||
.region("us-east-1")
|
||||
.api_key("my-api-key")
|
||||
.host_override("https://example.com")
|
||||
.execute()
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let (impl_name, properties) = conn.namespace_client_config().await.unwrap();
|
||||
assert_eq!(impl_name, "rest");
|
||||
assert_eq!(properties["uri"], "https://example.com");
|
||||
assert!(
|
||||
!properties.contains_key("header.x-lancedb-database"),
|
||||
"storage-root connections must not send the x-lancedb-database header"
|
||||
);
|
||||
|
||||
let result = ConnectBuilder::new("db://")
|
||||
.region("us-east-1")
|
||||
.api_key("my-api-key")
|
||||
.execute()
|
||||
.await;
|
||||
assert!(matches!(
|
||||
result,
|
||||
Err(Error::InvalidInput { message })
|
||||
if message.contains("A host override is required")
|
||||
));
|
||||
}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
#[tokio::test]
|
||||
async fn test_connect_rejects_header_provider_with_oauth_config() {
|
||||
|
||||
@@ -54,6 +54,7 @@
|
||||
//! - `/path/to/database` - local database on file system.
|
||||
//! - `s3://bucket/path/to/database` or `gs://bucket/path/to/database` - database on cloud object store
|
||||
//! - `db://dbname` - Lance Cloud
|
||||
//! - `db://` with a host override - remote tables in the storage root
|
||||
//!
|
||||
//! You can also use [`ConnectBuilder`] to configure the connection to the database.
|
||||
//!
|
||||
|
||||
@@ -378,23 +378,30 @@ pub struct ParsedDbUrl {
|
||||
|
||||
/// Parse a database URL and extract the database name and optional prefix.
|
||||
///
|
||||
/// Expected format: `db://db_name` or `db://db_name/prefix`
|
||||
/// Expected format: `db://db_name`, `db://db_name/prefix`, or `db://` when
|
||||
/// connecting to the storage root through a host override.
|
||||
pub fn parse_db_url(db_url: &str) -> Result<ParsedDbUrl> {
|
||||
let parsed_url = url::Url::parse(db_url).map_err(|err| Error::InvalidInput {
|
||||
message: format!("db_url is not a valid URL. '{db_url}'. Error: {err}"),
|
||||
})?;
|
||||
debug_assert_eq!(parsed_url.scheme(), "db");
|
||||
if !parsed_url.has_host() {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!("Invalid database URL (missing host) '{}'", db_url),
|
||||
});
|
||||
}
|
||||
let db_name = urlencoding::decode(parsed_url.host_str().unwrap())
|
||||
.map_err(|err| Error::InvalidInput {
|
||||
message: format!("Invalid encoded database name: {err}"),
|
||||
})?
|
||||
.into_owned();
|
||||
validate_database_name(&db_name)?;
|
||||
let db_name = match parsed_url.host_str() {
|
||||
Some(db_name) => {
|
||||
let db_name = urlencoding::decode(db_name)
|
||||
.map_err(|err| Error::InvalidInput {
|
||||
message: format!("Invalid encoded database name: {err}"),
|
||||
})?
|
||||
.into_owned();
|
||||
validate_database_name(&db_name)?;
|
||||
db_name
|
||||
}
|
||||
None if matches!(parsed_url.path(), "" | "/") => String::new(),
|
||||
None => {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!("Invalid database URL (missing host) '{}'", db_url),
|
||||
});
|
||||
}
|
||||
};
|
||||
let db_prefix = {
|
||||
let prefix = parsed_url.path().trim_start_matches('/');
|
||||
if prefix.is_empty() {
|
||||
|
||||
@@ -318,6 +318,13 @@ impl RemoteDatabase {
|
||||
read_consistency_interval: Option<std::time::Duration>,
|
||||
) -> Result<Self> {
|
||||
let parsed = super::client::parse_db_url(uri)?;
|
||||
if parsed.db_name.is_empty() && host_overrides.rest.is_none() {
|
||||
return Err(Error::InvalidInput {
|
||||
message:
|
||||
"A host override is required when connecting to the storage root with 'db://'"
|
||||
.to_string(),
|
||||
});
|
||||
}
|
||||
Self::try_new_with_identity(
|
||||
uri,
|
||||
api_key,
|
||||
|
||||
Reference in New Issue
Block a user