From 5b57991d69c017cf7bf9f7bbbb0a98b4ca71a7b5 Mon Sep 17 00:00:00 2001 From: Jonathan M Hsieh Date: Tue, 15 Sep 2026 21:50:01 +0000 Subject: [PATCH] refactor: one character set for every object name `TABLE_NAME_REGEX`, `NAMESPACE_NAME_REGEX` and `SECRET_NAME_REGEX` admitted the same characters, spelled three ways. Separate and identical is worse than either having one or having a reason to differ: a reader had to compare them to find out which it was, and they could drift without anyone noticing -- as they briefly had, since the Secret one carried a length bound the other two never did. `OBJECT_NAME_REGEX` replaces all three, beside the other shared validators, and is what a table name, a namespace segment, a Secret name and a materialized view name are all checked against. The validators stay separate, because what they do with a failure differs: a table reports `InvalidTableName`, the others `InvalidInput`, and the Secret one names whether it was the name or a path segment. Only the set is shared. `.` and `..` remain a separate predicate rather than folding into the pattern. A character set says what a name may contain; the relative-segment rule says where a name may sit. A pattern that tried to express both would have to permit `a..b`, `..a` and `a..` while refusing the two exact values -- which is how the regex suggested in review came to allow `.` and reject `a..b`, exactly inverting the rule. Function names are still unchecked in this client, and materialized views borrow the table validator. Both are worth revisiting, but neither is a change this refactor makes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6 --- rust/lancedb/src/utils/mod.rs | 43 ++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/rust/lancedb/src/utils/mod.rs b/rust/lancedb/src/utils/mod.rs index 32d54eba0..6e50472bf 100644 --- a/rust/lancedb/src/utils/mod.rs +++ b/rust/lancedb/src/utils/mod.rs @@ -19,10 +19,25 @@ use std::pin::Pin; use crate::error::{Error, Result}; use datafusion_physical_plan::SendableRecordBatchStream; -static TABLE_NAME_REGEX: std::sync::LazyLock = - std::sync::LazyLock::new(|| regex::Regex::new(r"^[a-zA-Z0-9_\-\.]+$").unwrap()); -static NAMESPACE_NAME_REGEX: std::sync::LazyLock = - std::sync::LazyLock::new(|| regex::Regex::new(r"^[a-zA-Z0-9_\-\.]+$").unwrap()); +/// The characters any object name may contain: a table, a namespace segment, a +/// Secret, a materialized view. +/// +/// One set rather than one per object type. They were separate and identical, +/// which is worse than either having one or having a reason to differ -- a +/// reader had to compare them to find out, and they could drift without anyone +/// noticing. +/// +/// No positional rule on top of it: a name may begin with `_`, `-` or `.`, +/// because LanceDB namespaces already do, and anything narrower would put +/// objects out of reach inside namespaces that already exist. `.` and `..` are +/// excluded separately, by [`reject_relative_segment`], because that is a +/// property of where a name sits in a URL rather than of the name itself. +/// +/// No length bound. How long a name may be is the service's to decide, and a +/// bound here could only disagree with it -- one that is shorter refuses names +/// the catalog would hold, and one that is longer says nothing. +static OBJECT_NAME_REGEX: std::sync::LazyLock = + std::sync::LazyLock::new(|| regex::Regex::new(r"^[A-Za-z0-9_.\-]+$").unwrap()); pub trait PatchStoreParam { fn patch_with_store_wrapper( @@ -115,7 +130,7 @@ pub fn validate_table_name(name: &str) -> Result<()> { reason: "Table names cannot be empty strings".to_string(), }); } - if !TABLE_NAME_REGEX.is_match(name) { + if !OBJECT_NAME_REGEX.is_match(name) { return Err(Error::InvalidTableName { name: name.to_string(), reason: @@ -145,7 +160,7 @@ pub fn validate_namespace_name(name: &str) -> Result<()> { message: "Namespace names cannot be empty strings".to_string(), }); } - if !NAMESPACE_NAME_REGEX.is_match(name) { + if !OBJECT_NAME_REGEX.is_match(name) { return Err(Error::InvalidInput { message: format!( "Invalid namespace name '{}': Namespace names can only contain alphanumeric characters, underscores, hyphens, and periods", @@ -157,20 +172,6 @@ pub fn validate_namespace_name(name: &str) -> Result<()> { Ok(()) } -/// The characters a Secret name or a Secret namespace segment may contain. -/// -/// Deliberately the set the service admits, and no positional rule on top of -/// it: a segment may begin with `_`, `-` or `.`, because LanceDB namespaces -/// already do, and anything narrower would put Secrets out of reach inside -/// namespaces that already exist. -/// -/// No length bound either. How long a name may be is the service's to decide, -/// the way it is for a table, and a bound here could only disagree with it -- -/// one that is shorter refuses names the catalog would hold, and one that is -/// longer says nothing. -static SECRET_NAME_REGEX: std::sync::LazyLock = - std::sync::LazyLock::new(|| regex::Regex::new(r"^[A-Za-z0-9_.\-]+$").unwrap()); - /// Validate one component of a Secret identifier: a Secret name, or one segment /// of the namespace path holding it. /// @@ -202,7 +203,7 @@ pub fn validate_secret_component(what: &str, value: &str) -> Result<()> { message: format!("{what} must not be empty"), }); } - if !SECRET_NAME_REGEX.is_match(value) { + if !OBJECT_NAME_REGEX.is_match(value) { return Err(Error::InvalidInput { message: format!( "invalid {what} '{value}': it may contain only alphanumeric characters, \