diff --git a/nodejs/src/remote.rs b/nodejs/src/remote.rs index c619aca35..c1f0836fd 100644 --- a/nodejs/src/remote.rs +++ b/nodejs/src/remote.rs @@ -93,6 +93,9 @@ pub struct ClientConfig { pub retry_config: Option, pub timeout_config: Option, pub extra_headers: Option>, + /// The delimiter joining a namespace path and a name into one object + /// identifier. `"$"` is the only supported value, and leaving this unset is + /// how to get it; anything else is rejected when the connection is created. pub id_delimiter: Option, pub tls_config: Option, /// User identifier for tracking purposes. diff --git a/python/python/lancedb/remote/__init__.py b/python/python/lancedb/remote/__init__.py index 602d8dbab..adcab0f53 100644 --- a/python/python/lancedb/remote/__init__.py +++ b/python/python/lancedb/remote/__init__.py @@ -166,7 +166,10 @@ class ClientConfig: extra_headers: Optional[dict] Additional headers to include in requests. id_delimiter: Optional[str] - The delimiter to use when constructing object identifiers. + The delimiter joining a namespace path and a name into one object + identifier. ``"$"`` is the only supported value, and leaving this + unset is how to get it; anything else is rejected when the connection + is created. tls_config: Optional[TlsConfig] TLS/mTLS configuration for secure connections. header_provider: Optional[HeaderProvider] diff --git a/rust/lancedb/src/remote/client.rs b/rust/lancedb/src/remote/client.rs index 512eaa977..8ddf69046 100644 --- a/rust/lancedb/src/remote/client.rs +++ b/rust/lancedb/src/remote/client.rs @@ -90,8 +90,9 @@ pub struct ClientConfig { pub user_agent: String, // TODO: how to configure request ids? pub extra_headers: HashMap, - /// The delimiter to use when constructing object identifiers. - /// If not default, passes as query parameter. + /// The delimiter joining a namespace path and a name into one object + /// identifier. [`ID_DELIMITER`] is the only accepted value; any other is + /// refused by [`ClientConfig::validate`]. pub id_delimiter: Option, /// TLS configuration for mTLS support pub tls_config: Option, @@ -307,7 +308,6 @@ pub struct RestfulLanceDbClient { host: String, pub(crate) retry_config: ResolvedRetryConfig, pub(crate) sender: S, - pub(crate) id_delimiter: String, pub(crate) header_provider: Option>, /// Connection-level read consistency interval. Drives the /// `x-lancedb-min-timestamp` freshness header sent on read requests. @@ -330,7 +330,6 @@ impl std::fmt::Debug for RestfulLanceDbClient { .field("host", &self.host) .field("retry_config", &self.retry_config) .field("sender", &self.sender) - .field("id_delimiter", &self.id_delimiter) .field( "header_provider", &self.header_provider.as_ref().map(|_| "Some(...)"), @@ -453,39 +452,26 @@ enum BodyLogging { /// /// An empty delimiter is refused for the same reason from the other side: it /// joins `["a", "b"]` into `ab`, which no split recovers. -fn validate_id_delimiter(delimiter: String) -> Result { - if delimiter.is_empty() { - return Err(Error::InvalidInput { - message: "id_delimiter must not be empty: an identifier joined by nothing cannot be \ - split back into a namespace path and a name" - .to_string(), - }); - } - // The root namespace is addressed by the bare delimiter, so a delimiter that - // is itself a relative segment makes the root listing route disappear: - // `/v1/namespace/./secret/list` resolves to `/v1/namespace/secret/list`. - if delimiter == "." || delimiter == ".." { +/// The delimiter joining a namespace path and a name into the `{id}` a route +/// addresses, and the only one a LanceDB service splits on. +/// +/// The identifier grammar comes from the Lance REST catalog standard, which +/// carries a delimiter setting so other catalogs can adopt it; LanceDB itself +/// has one value. Fixing it here is what lets an identifier be joined and split +/// back apart without further checks: `$` is outside the character set every +/// object name admits, so no name can contain one. +pub(crate) const ID_DELIMITER: &str = "$"; + +fn validate_id_delimiter(delimiter: &str) -> Result<()> { + if delimiter != ID_DELIMITER { return Err(Error::InvalidInput { message: format!( - "id_delimiter '{delimiter}' is a relative path segment: the root namespace is \ - addressed by the delimiter alone, so a request for it would resolve to a \ - different route" + "id_delimiter '{delimiter}' is not supported: '{ID_DELIMITER}' is the only \ + delimiter LanceDB services split an identifier on" ), }); } - if let Some(unsafe_character) = delimiter - .chars() - .find(|character| matches!(character, '/' | '?' | '#' | '%' | '\\')) - { - return Err(Error::InvalidInput { - message: format!( - "id_delimiter '{delimiter}' contains {unsafe_character:?}, which is URL path \ - syntax: a delimiter is written into the path raw, so one that divides or ends \ - the path changes which route a request reaches" - ), - }); - } - Ok(delimiter) + Ok(()) } impl ClientConfig { @@ -496,7 +482,9 @@ impl ClientConfig { /// response later. Public so a caller assembling a config can ask the same /// question without connecting. pub fn validate(&self) -> Result<()> { - validate_id_delimiter(self.id_delimiter.clone().unwrap_or_else(|| "$".to_string()))?; + if let Some(delimiter) = &self.id_delimiter { + validate_id_delimiter(delimiter)?; + } Ok(()) } } @@ -629,10 +617,6 @@ impl RestfulLanceDbClient { host, retry_config, sender: Sender, - id_delimiter: client_config - .id_delimiter - .clone() - .unwrap_or("$".to_string()), header_provider: client_config.header_provider, read_consistency_interval, max_bytes_per_request, @@ -790,22 +774,12 @@ impl RestfulLanceDbClient { pub fn get(&self, uri: &str) -> RequestBuilder { let full_uri = format!("{}{}", self.host, uri); - let builder = self.client.get(full_uri); - self.add_id_delimiter_query_param(builder) + self.client.get(full_uri) } pub fn post(&self, uri: &str) -> RequestBuilder { let full_uri = format!("{}{}", self.host, uri); - let builder = self.client.post(full_uri); - self.add_id_delimiter_query_param(builder) - } - - fn add_id_delimiter_query_param(&self, req: RequestBuilder) -> RequestBuilder { - if self.id_delimiter != "$" { - req.query(&[("delimiter", self.id_delimiter.clone())]) - } else { - req - } + self.client.post(full_uri) } /// Apply dynamic headers from the header provider if configured @@ -1148,7 +1122,6 @@ pub mod test_utils { sender: MockSender { f: Arc::new(wrapper), }, - id_delimiter: "$".to_string(), header_provider: None, read_consistency_interval, max_bytes_per_request: None, @@ -1175,7 +1148,6 @@ pub mod test_utils { sender: MockSender { f: Arc::new(wrapper), }, - id_delimiter: config.id_delimiter.unwrap_or_else(|| "$".to_string()), header_provider: config.header_provider, read_consistency_interval: None, max_bytes_per_request: config @@ -1190,27 +1162,38 @@ pub mod test_utils { #[cfg(test)] mod tests { - /// A delimiter is written into the path raw, so one that is path syntax - /// changes the shape of a route rather than its content. Refusing a - /// component that contains the delimiter cannot catch this: neither - /// component contains one. + /// The identifier grammar has one delimiter. A configuration naming any + /// other is refused where it was written, rather than producing identifiers + /// no service would split the way the caller meant. #[test] - fn test_a_delimiter_that_is_path_syntax_is_refused() { - for delimiter in ["/", "?", "#", "%", "a/b", "", ".", "..", "/../../function/"] { - let error = super::validate_id_delimiter(delimiter.to_string()) - .expect_err("a delimiter that is path syntax must be refused"); + fn test_a_delimiter_other_than_the_supported_one_is_refused() { + for delimiter in ["/", "?", "#", "%", "", ".", "..", "-", "_", "|", "::", "$$"] { + let error = super::validate_id_delimiter(delimiter) + .expect_err("only the supported delimiter may be configured"); assert!( error.to_string().contains("id_delimiter"), "{delimiter:?}: {error}" ); } - // The default, and other separators that are not path syntax, stand. - // A period is fine inside a longer delimiter -- only the exact relative - // segments are a problem. - for delimiter in ["$", "-", "_", "|", "::", ".x.", "..."] { - super::validate_id_delimiter(delimiter.to_string()) - .unwrap_or_else(|error| panic!("{delimiter:?} must be accepted: {error}")); + super::validate_id_delimiter(super::ID_DELIMITER).unwrap(); + } + + /// Leaving it unset is how nearly every caller reaches the same delimiter. + #[test] + fn test_an_unset_delimiter_is_the_supported_one() { + super::ClientConfig::default().validate().unwrap(); + super::ClientConfig { + id_delimiter: Some(super::ID_DELIMITER.to_string()), + ..Default::default() } + .validate() + .unwrap(); + super::ClientConfig { + id_delimiter: Some("-".to_string()), + ..Default::default() + } + .validate() + .expect_err("a configured delimiter other than the supported one must be refused"); } use super::*; @@ -1513,7 +1496,6 @@ mod tests { host: "https://example.com".to_string(), retry_config: RetryConfig::default().try_into().unwrap(), sender: Sender, - id_delimiter: "+".to_string(), header_provider: Some(Arc::new(provider) as Arc), read_consistency_interval: None, max_bytes_per_request: None, @@ -1552,7 +1534,6 @@ mod tests { host: "https://example.com".to_string(), retry_config: RetryConfig::default().try_into().unwrap(), sender: Sender, - id_delimiter: "+".to_string(), header_provider: Some(Arc::new(provider) as Arc), read_consistency_interval: None, max_bytes_per_request: None, @@ -1617,7 +1598,6 @@ mod tests { host: "https://example.com".to_string(), retry_config: RetryConfig::default().try_into().unwrap(), sender: Sender, - id_delimiter: "+".to_string(), header_provider: Some(Arc::new(provider) as Arc), read_consistency_interval: None, max_bytes_per_request: None, diff --git a/rust/lancedb/src/remote/db.rs b/rust/lancedb/src/remote/db.rs index 308c3c198..dcaa2426f 100644 --- a/rust/lancedb/src/remote/db.rs +++ b/rust/lancedb/src/remote/db.rs @@ -37,7 +37,8 @@ use crate::table::BaseTable; use crate::utils::{reject_relative_segment, validate_namespace_name, validate_table_name}; use super::client::{ - ClientConfig, HeaderProvider, HttpSend, RequestResultExt, RestfulLanceDbClient, Sender, + ClientConfig, HeaderProvider, HttpSend, ID_DELIMITER, RequestResultExt, RestfulLanceDbClient, + Sender, }; use super::sql::SqlClient; use super::table::RemoteTable; @@ -381,7 +382,7 @@ impl RemoteDatabase { name: &str, namespace_path: &[String], ) -> Result<(String, Response)> { - let identifier = build_table_identifier(name, namespace_path, &self.client.id_delimiter)?; + let identifier = build_table_identifier(name, namespace_path)?; let cache_key = build_cache_key(name, namespace_path); let req = self.client.post(&format!("/v1/table/{}/drop/", identifier)); let (request_id, resp) = self.client.send(req).await?; @@ -405,8 +406,7 @@ impl RemoteDatabase { &self, request: &TableNamesRequest, ) -> Result<(Vec, ServerVersion)> { - let namespace_id = - build_namespace_identifier(&request.namespace_path, &self.client.id_delimiter)?; + let namespace_id = build_namespace_identifier(&request.namespace_path)?; let path = format!("/v1/namespace/{}/table/list", namespace_id); let mut names = Vec::new(); @@ -514,16 +514,13 @@ impl From<&CreateTableMode> for &'static str { } /// The path segment addressing one table: its namespace path and name. -fn build_table_identifier(name: &str, namespace: &[String], delimiter: &str) -> Result { +fn build_table_identifier(name: &str, namespace: &[String]) -> Result { for segment in namespace { validate_namespace_name(segment)?; - reject_delimiter(segment, delimiter, "namespace segment")?; } validate_table_name(name)?; - reject_delimiter(name, delimiter, "table name")?; Ok(join_identifier( namespace.iter().map(String::as_str).chain([name]), - delimiter, )) } @@ -534,33 +531,7 @@ fn build_table_identifier(name: &str, namespace: &[String], delimiter: &str) -> /// untouched and is then resolved away. fn build_function_identifier(name: &str) -> Result { reject_relative_segment("Function name", name)?; - Ok(join_identifier([name].into_iter(), "")) -} - -/// Refuse a component holding the delimiter that is about to separate it from -/// the others. -/// -/// The join is what decides identity, and it is only reversible while no -/// component contains the separator. The default `$` is outside every name -/// charset here, so this never fires for it. A *configured* delimiter may be -/// any string, and the readable choices -- `.`, `-`, `_` -- are all inside -/// those charsets: under `id_delimiter="."` the namespace `["prod.vision"]` -/// and the namespace `["prod", "vision"]` join to one identifier, and a caller -/// addressing the first would reach the object belonging to the second. -/// -/// There is no escaping of the joined form that recovers a boundary the -/// delimiter has already erased, so this refuses rather than encodes. -fn reject_delimiter(component: &str, delimiter: &str, what: &str) -> Result<()> { - if !delimiter.is_empty() && component.contains(delimiter) { - return Err(Error::InvalidInput { - message: format!( - "{what} '{component}' contains the configured identifier delimiter \ - '{delimiter}', so the namespace path and the name it joins could not be told \ - apart" - ), - }); - } - Ok(()) + Ok(join_identifier([name].into_iter())) } /// The path segment addressing one Secret: its namespace path and name. @@ -570,15 +541,8 @@ fn reject_delimiter(component: &str, delimiter: &str, what: &str) -> Result<()> /// point. /// /// [`Connection`]: crate::connection::Connection -fn build_secret_identifier(name: &str, namespace: &[String], delimiter: &str) -> Result { - for segment in namespace { - reject_delimiter(segment, delimiter, "Secret namespace path segment")?; - } - reject_delimiter(name, delimiter, "Secret name")?; - Ok(join_identifier( - namespace.iter().map(String::as_str).chain([name]), - delimiter, - )) +fn build_secret_identifier(name: &str, namespace: &[String]) -> String { + join_identifier(namespace.iter().map(String::as_str).chain([name])) } /// Join validated components into the `{id}` a route addresses: each @@ -594,43 +558,33 @@ fn build_secret_identifier(name: &str, namespace: &[String], delimiter: &str) -> /// URL's shape. It does not cover `.` and `..`, which are unreserved too and /// are resolved after decoding -- those are refused outright, by the validator /// each object has. -fn join_identifier<'a>(components: impl Iterator, delimiter: &str) -> String { +fn join_identifier<'a>(components: impl Iterator) -> String { components .map(|component| urlencoding::encode(component).into_owned()) .collect::>() - .join(delimiter) + .join(ID_DELIMITER) } /// The namespace a Secret listing is scoped to. /// /// The root is the bare delimiter, as it is for every other object's listing. -fn build_secret_namespace_identifier(namespace: &[String], delimiter: &str) -> Result { - for segment in namespace { - reject_delimiter(segment, delimiter, "Secret namespace path segment")?; - } +fn build_secret_namespace_identifier(namespace: &[String]) -> String { if namespace.is_empty() { - return Ok(delimiter.to_string()); + return ID_DELIMITER.to_string(); } - Ok(join_identifier( - namespace.iter().map(String::as_str), - delimiter, - )) + join_identifier(namespace.iter().map(String::as_str)) } /// The path segment addressing one namespace. -fn build_namespace_identifier(namespace: &[String], delimiter: &str) -> Result { +fn build_namespace_identifier(namespace: &[String]) -> Result { for segment in namespace { validate_namespace_name(segment)?; - reject_delimiter(segment, delimiter, "namespace segment")?; } if namespace.is_empty() { // According to the namespace spec, use delimiter to represent root namespace - return Ok(delimiter.to_string()); + return Ok(ID_DELIMITER.to_string()); } - Ok(join_identifier( - namespace.iter().map(String::as_str), - delimiter, - )) + Ok(join_identifier(namespace.iter().map(String::as_str))) } /// Build a secure cache key using length prefixes. @@ -767,11 +721,7 @@ impl Database for RemoteDatabase { &self, request: CreateMaterializedViewRequest, ) -> Result { - let identifier = build_table_identifier( - &request.name, - &request.namespace_path, - &self.client.id_delimiter, - )?; + let identifier = build_table_identifier(&request.name, &request.namespace_path)?; let req = self .client .post(&format!("/v1/materialized_view/{identifier}/create")) @@ -814,7 +764,7 @@ impl Database for RemoteDatabase { name: &str, namespace_path: &[String], ) -> Result { - let identifier = build_table_identifier(name, namespace_path, &self.client.id_delimiter)?; + let identifier = build_table_identifier(name, namespace_path)?; let request = self .client .post(&format!("/v1/materialized_view/{identifier}/drop")); @@ -852,7 +802,7 @@ impl Database for RemoteDatabase { page_token: Option, } - let namespace_id = build_namespace_identifier(namespace_path, &self.client.id_delimiter)?; + let namespace_id = build_namespace_identifier(namespace_path)?; let path = format!("/v1/namespace/{namespace_id}/materialized_view/list"); let mut views = Vec::new(); let mut page_token: Option = None; @@ -927,7 +877,7 @@ impl Database for RemoteDatabase { } async fn list_functions(&self) -> Result> { - let namespace_id = build_namespace_identifier(&[], &self.client.id_delimiter)?; + let namespace_id = build_namespace_identifier(&[])?; let path = format!("/v1/namespace/{namespace_id}/function/list"); let mut functions = Vec::new(); let mut page_token: Option = None; @@ -987,7 +937,7 @@ impl Database for RemoteDatabase { value: &str, namespace_path: &[String], ) -> Result<()> { - let secret_id = build_secret_identifier(name, namespace_path, &self.client.id_delimiter)?; + let secret_id = build_secret_identifier(name, namespace_path); self.post_secret_write( &format!("/v1/secret/{secret_id}/create"), &RemoteCreateSecretRequest { value }, @@ -996,7 +946,7 @@ impl Database for RemoteDatabase { } async fn alter_secret(&self, name: &str, value: &str, namespace_path: &[String]) -> Result<()> { - let secret_id = build_secret_identifier(name, namespace_path, &self.client.id_delimiter)?; + let secret_id = build_secret_identifier(name, namespace_path); self.post_secret_write( &format!("/v1/secret/{secret_id}/alter"), &RemoteAlterSecretRequest { value }, @@ -1005,8 +955,7 @@ impl Database for RemoteDatabase { } async fn list_secrets(&self, namespace_path: &[String]) -> Result> { - let namespace_id = - build_secret_namespace_identifier(namespace_path, &self.client.id_delimiter)?; + let namespace_id = build_secret_namespace_identifier(namespace_path); let path = format!("/v1/namespace/{namespace_id}/secret/list"); let mut names = Vec::new(); let mut page_token: Option = None; @@ -1039,7 +988,7 @@ impl Database for RemoteDatabase { } async fn drop_secret(&self, name: &str, namespace_path: &[String]) -> Result<()> { - let secret_id = build_secret_identifier(name, namespace_path, &self.client.id_delimiter)?; + let secret_id = build_secret_identifier(name, namespace_path); let req = self.client.post(&format!("/v1/secret/{secret_id}/drop")); let (request_id, response) = self.client.send(req).await?; self.client.check_response(&request_id, response).await?; @@ -1047,7 +996,7 @@ impl Database for RemoteDatabase { } async fn describe_secret(&self, name: &str, namespace_path: &[String]) -> Result { - let secret_id = build_secret_identifier(name, namespace_path, &self.client.id_delimiter)?; + let secret_id = build_secret_identifier(name, namespace_path); let req = self .client .post(&format!("/v1/secret/{secret_id}/describe")); @@ -1178,8 +1127,7 @@ impl Database for RemoteDatabase { }; for table in &tables { - let table_identifier = - build_table_identifier(table, &request.namespace_path, &self.client.id_delimiter)?; + let table_identifier = build_table_identifier(table, &request.namespace_path)?; let cache_key = build_cache_key(table, &request.namespace_path); let remote_table = Arc::new(RemoteTable::new( self.client.clone(), @@ -1195,7 +1143,7 @@ impl Database for RemoteDatabase { async fn list_tables(&self, request: ListTablesRequest) -> Result { let namespace_parts = request.id.as_deref().unwrap_or(&[]); - let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?; + let namespace_id = build_namespace_identifier(namespace_parts)?; let mut req = self .client .get(&format!("/v1/namespace/{}/table/list", namespace_id)); @@ -1215,8 +1163,7 @@ impl Database for RemoteDatabase { // Cache the tables for future use let namespace_vec = namespace_parts.to_vec(); for table in &response.tables { - let table_identifier = - build_table_identifier(table, &namespace_vec, &self.client.id_delimiter)?; + let table_identifier = build_table_identifier(table, &namespace_vec)?; let cache_key = build_cache_key(table, &namespace_vec); let remote_table = Arc::new(RemoteTable::new( self.client.clone(), @@ -1234,11 +1181,7 @@ impl Database for RemoteDatabase { async fn create_table(&self, mut request: CreateTableRequest) -> Result> { let body = stream_as_body(request.data.scan_as_stream())?; - let identifier = build_table_identifier( - &request.name, - &request.namespace_path, - &self.client.id_delimiter, - )?; + let identifier = build_table_identifier(&request.name, &request.namespace_path)?; let req = self .client .post(&format!("/v1/table/{}/create/", identifier)) @@ -1290,11 +1233,7 @@ impl Database for RemoteDatabase { } let rsp = self.client.check_response(&request_id, rsp).await?; let version = parse_server_version(&request_id, &rsp)?; - let table_identifier = build_table_identifier( - &request.name, - &request.namespace_path, - &self.client.id_delimiter, - )?; + let table_identifier = build_table_identifier(&request.name, &request.namespace_path)?; let cache_key = build_cache_key(&request.name, &request.namespace_path); let table = Arc::new(RemoteTable::new( self.client.clone(), @@ -1309,11 +1248,8 @@ impl Database for RemoteDatabase { } async fn clone_table(&self, request: CloneTableRequest) -> Result> { - let table_identifier = build_table_identifier( - &request.target_table_name, - &request.target_namespace_path, - &self.client.id_delimiter, - )?; + let table_identifier = + build_table_identifier(&request.target_table_name, &request.target_namespace_path)?; let remote_request = RemoteCloneTableRequest { source_location: request.source_uri, @@ -1354,11 +1290,7 @@ impl Database for RemoteDatabase { } async fn open_table(&self, request: OpenTableRequest) -> Result> { - let identifier = build_table_identifier( - &request.name, - &request.namespace_path, - &self.client.id_delimiter, - )?; + let identifier = build_table_identifier(&request.name, &request.namespace_path)?; let cache_key = build_cache_key(&request.name, &request.namespace_path); // We describe the table to confirm it exists before moving on. @@ -1374,11 +1306,7 @@ impl Database for RemoteDatabase { let rsp = self.client.check_response(&request_id, rsp).await?; let version = parse_server_version(&request_id, &rsp)?; let describe_body = rsp.text().await.ok(); - let table_identifier = build_table_identifier( - &request.name, - &request.namespace_path, - &self.client.id_delimiter, - )?; + let table_identifier = build_table_identifier(&request.name, &request.namespace_path)?; let table = Arc::new(RemoteTable::new( self.client.clone(), request.name.clone(), @@ -1405,8 +1333,7 @@ impl Database for RemoteDatabase { cur_namespace_path: &[String], new_namespace_path: &[String], ) -> Result<()> { - let current_identifier = - build_table_identifier(current_name, cur_namespace_path, &self.client.id_delimiter)?; + let current_identifier = build_table_identifier(current_name, cur_namespace_path)?; let current_cache_key = build_cache_key(current_name, cur_namespace_path); let new_cache_key = build_cache_key(new_name, new_namespace_path); @@ -1470,7 +1397,7 @@ impl Database for RemoteDatabase { request: ListNamespacesRequest, ) -> Result { let namespace_parts = request.id.as_deref().unwrap_or(&[]); - let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?; + let namespace_id = build_namespace_identifier(namespace_parts)?; let mut req = self .client .get(&format!("/v1/namespace/{}/list", namespace_id)); @@ -1492,7 +1419,7 @@ impl Database for RemoteDatabase { request: CreateNamespaceRequest, ) -> Result { let namespace_parts = request.id.as_deref().unwrap_or(&[]); - let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?; + let namespace_id = build_namespace_identifier(namespace_parts)?; let mut req = self .client .post(&format!("/v1/namespace/{}/create", namespace_id)); @@ -1520,7 +1447,7 @@ impl Database for RemoteDatabase { async fn drop_namespace(&self, request: DropNamespaceRequest) -> Result { let namespace_parts = request.id.as_deref().unwrap_or(&[]); - let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?; + let namespace_id = build_namespace_identifier(namespace_parts)?; let mut req = self .client .post(&format!("/v1/namespace/{}/drop", namespace_id)); @@ -1551,7 +1478,7 @@ impl Database for RemoteDatabase { request: DescribeNamespaceRequest, ) -> Result { let namespace_parts = request.id.as_deref().unwrap_or(&[]); - let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?; + let namespace_id = build_namespace_identifier(namespace_parts)?; let req = self .client .post(&format!("/v1/namespace/{}/describe", namespace_id)) @@ -1570,7 +1497,7 @@ impl Database for RemoteDatabase { async fn namespace_client(&self) -> Result> { // Create a RestNamespace pointing to the same remote host with the same authentication headers let mut builder = lance_namespace_impls::RestNamespaceBuilder::new(self.client.host()) - .delimiter(&self.client.id_delimiter) + .delimiter(ID_DELIMITER) .headers(self.namespace_headers.clone()); if let Some(context_provider) = &self.namespace_context_provider { @@ -1606,7 +1533,7 @@ impl Database for RemoteDatabase { let mut properties = HashMap::new(); properties.insert("uri".to_string(), self.client.host().to_string()); - properties.insert("delimiter".to_string(), self.client.id_delimiter.clone()); + properties.insert("delimiter".to_string(), ID_DELIMITER.to_string()); for (key, value) in &self.namespace_headers { properties.insert(format!("header.{}", key), value.clone()); } @@ -3507,57 +3434,6 @@ mod tests { } } - #[tokio::test] - async fn test_a_configured_delimiter_is_refused_inside_a_component() { - use std::sync::{Arc, Mutex}; - let reached = Arc::new(Mutex::new(false)); - let flag = reached.clone(); - let conn = Connection::new_with_handler_and_config( - move |_| { - *flag.lock().unwrap() = true; - http::Response::builder().status(200).body("{}").unwrap() - }, - ClientConfig { - id_delimiter: Some(".".to_string()), - ..Default::default() - }, - ); - let error = conn - .drop_secret("openai", &["prod.vision".to_string()]) - .await - .expect_err("a component holding the delimiter must be refused"); - assert!(error.to_string().contains("delimiter"), "{error}"); - assert!(!*reached.lock().unwrap(), "the request was sent anyway"); - - // The same two names as separate segments are the ordinary case. - conn.drop_secret("openai", &["prod".to_string(), "vision".to_string()]) - .await - .unwrap(); - } - - #[tokio::test] - async fn test_a_configured_delimiter_is_refused_inside_a_table_component() { - use std::sync::{Arc, Mutex}; - let reached = Arc::new(Mutex::new(false)); - let flag = reached.clone(); - let conn = Connection::new_with_handler_and_config( - move |_| { - *flag.lock().unwrap() = true; - http::Response::builder().status(200).body("{}").unwrap() - }, - ClientConfig { - id_delimiter: Some(".".to_string()), - ..Default::default() - }, - ); - let error = conn - .drop_table("t", &["prod.vision".to_string()]) - .await - .expect_err("a segment holding the delimiter must be refused"); - assert!(error.to_string().contains("delimiter"), "{error}"); - assert!(!*reached.lock().unwrap(), "the request was sent anyway"); - } - #[tokio::test] async fn test_create_and_alter_secret_send_the_value_in_the_request_body() { for (route, call) in [