From ef681c0c30e97fa5d8886757bbeb7b2f6474e5a5 Mon Sep 17 00:00:00 2001 From: Jack Ye Date: Tue, 22 Sep 2026 21:32:47 -0700 Subject: [PATCH] fix: tell a caller their view name is invalid, not their table name `validate_view_reference` deferred to `validate_table_name`, so a view name carrying a `$` came back as "Invalid table name" -- an object the caller never named. Views get the treatment Secrets already have: a validator worded for the object, in utils beside its siblings. --- rust/lancedb/src/connection.rs | 13 ++----------- rust/lancedb/src/remote/db.rs | 9 +++++++-- rust/lancedb/src/utils/mod.rs | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/rust/lancedb/src/connection.rs b/rust/lancedb/src/connection.rs index 2e29e32a0..0a5ddbef5 100644 --- a/rust/lancedb/src/connection.rs +++ b/rust/lancedb/src/connection.rs @@ -38,7 +38,8 @@ use crate::remote::{ }; use crate::secrets::SecretInfo; use crate::utils::{ - validate_namespace, validate_secret_component, validate_secret_reference, validate_table_name, + validate_namespace, validate_secret_component, validate_secret_reference, + validate_view_reference, }; use crate::view::ViewDescription; use lance::io::ObjectStoreParams; @@ -416,16 +417,6 @@ impl CloneTableBuilder { } } -/// Validate a view name and every segment of the namespace path holding it. -/// -/// A view is addressed by the two joined into one identifier, so a segment -/// carrying the delimiter would split back apart as a different view. Neither -/// name accepts one, which is what lets the join have one meaning. -fn validate_view_reference(name: &str, namespace_path: &[String]) -> Result<()> { - validate_namespace(namespace_path)?; - validate_table_name(name) -} - /// A connection to LanceDB #[derive(Clone)] pub struct Connection { diff --git a/rust/lancedb/src/remote/db.rs b/rust/lancedb/src/remote/db.rs index 02a5b3b80..893c584e1 100644 --- a/rust/lancedb/src/remote/db.rs +++ b/rust/lancedb/src/remote/db.rs @@ -4073,9 +4073,14 @@ mod tests { ("", vec![]), ("..", vec![]), ] { + let error = match conn.describe_view(name, &namespace).await { + Ok(_) => panic!("accepted {name:?} in {namespace:?}"), + Err(error) => error.to_string(), + }; + // A view is not a table, so the refusal says so. assert!( - conn.describe_view(name, &namespace).await.is_err(), - "accepted {name:?} in {namespace:?}" + error.contains("view name") || error.contains("view namespace path segment"), + "{error}" ); } } diff --git a/rust/lancedb/src/utils/mod.rs b/rust/lancedb/src/utils/mod.rs index f80cc876a..52b7bd5e8 100644 --- a/rust/lancedb/src/utils/mod.rs +++ b/rust/lancedb/src/utils/mod.rs @@ -174,6 +174,27 @@ pub fn validate_secret_reference(name: &str, namespace_path: &[String]) -> Resul validate_secret_component("Secret name", name) } +/// Validate a view name and every segment of the namespace path holding it. +/// +/// Worded for a view rather than deferring to [`validate_table_name`]: a view +/// is not a table, and a caller who mistypes one should not be told their +/// table name is invalid. The rule is the same one every object name follows, +/// which is what lets the `$`-joined identifier split back apart. +pub fn validate_view_reference(name: &str, namespace_path: &[String]) -> Result<()> { + for segment in namespace_path { + validate_view_component("view namespace path segment", segment)?; + } + validate_view_component("view name", name) +} + +/// Validate one component of a view identifier: a view name, or one segment of +/// the namespace path holding it. +pub fn validate_view_component(what: &str, value: &str) -> Result<()> { + check_object_name(value).map_err(|reason| Error::InvalidInput { + message: format!("invalid {what} '{value}': {reason}"), + }) +} + /// Validate all components of a namespace /// /// Iterates through all namespace components and validates each one.