From aea54f8d33e116a58d5d78697165c3c7184b5c0d Mon Sep 17 00:00:00 2001 From: Filipriec Date: Tue, 25 Aug 2026 13:01:32 +0200 Subject: [PATCH] table name --- AGENTS.md | 8 +++ Cargo.toml | 2 +- common/src/alias.rs | 12 ++++ search/src/lib.rs | 99 ++++++++++++++++++++----------- server | 2 +- web/locales/cs/main.ftl | 2 + web/locales/en/main.ftl | 2 + web/locales/sk/main.ftl | 2 + web/src/pages/add_table/draft.rs | 10 +++- web/src/pages/add_table/loader.rs | 2 +- web/src/pages/add_table/state.rs | 2 +- web/src/schema/mod.rs | 37 ++++++++++-- 12 files changed, 135 insertions(+), 45 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 03ca2431..b321b984 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,3 +2,11 @@ - Never run `cargo fmt`, `rustfmt`, or any other automatic code-formatting command in this repository, including commands scoped to individual files or packages. - Preserve existing formatting. Make only the smallest hand-edited changes required for the task. + +## Long-running commands + +- Never repeatedly poll a running command. +- Wait at most 10 seconds initially. If the command is still running, leave it running, report the command and session ID, and end the turn immediately. +- Do not poll the command again unless the user explicitly asks for its status. +- Do not start optional slow checks. +- Before starting a required command that cannot safely be left running, ask the user for permission. diff --git a/Cargo.toml b/Cargo.toml index 56db47d2..5cdc8d82 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -members = ["client", "server", "common", "search", "tui-canvas", "tui-canvas/tui-canvas-validation-core", "tui-pages" ] +members = ["client", "client-gui", "server", "common", "search", "tui-canvas", "tui-canvas/tui-canvas-validation-core", "tui-pages" ] resolver = "3" [workspace.package] diff --git a/common/src/alias.rs b/common/src/alias.rs index 361e9478..fb45f071 100644 --- a/common/src/alias.rs +++ b/common/src/alias.rs @@ -9,6 +9,13 @@ use unicode_properties::{GeneralCategoryGroup, UnicodeEmoji, UnicodeGeneralCateg /// labels in a few query paths, so the limit is measured in UTF-8 bytes. pub const MAX_ALIAS_BYTES: usize = 63; +/// The case-insensitive key used to compare public table names. Table names +/// have a deliberately small ASCII vocabulary, so PostgreSQL's `lower()` and +/// this function produce the same stored key. +pub fn canonical_table_name(table_name: &str) -> String { + table_name.to_ascii_lowercase() +} + /// Whether a display alias contains control, formatting, private-use, or /// unassigned characters that should never be persisted as visible naming. pub fn has_disallowed_alias_characters(alias: &str) -> bool { @@ -68,6 +75,11 @@ pub fn canonical_alias(alias: &str) -> String { mod tests { use super::*; + #[test] + fn table_names_compare_without_ascii_case() { + assert_eq!(canonical_table_name("Sales-2026_Q4"), "sales-2026_q4"); + } + #[test] fn aliases_compare_by_compatibility_case_and_without_emoji() { assert_eq!(canonical_alias("Apple"), "apple"); diff --git a/search/src/lib.rs b/search/src/lib.rs index 7a042fb6..a3825fb5 100644 --- a/search/src/lib.rs +++ b/search/src/lib.rs @@ -165,7 +165,7 @@ impl SearcherService { request: Request, ) -> Result, Status> { let req = request.into_inner(); - let normalized = normalize_request(req)?; + let mut normalized = normalize_request(req)?; if !profile_exists(&self.pool, &normalized.profile_name).await? { return Err(Status::not_found(format!( @@ -174,13 +174,15 @@ impl SearcherService { ))); } - if let Some(table_name) = normalized.table_name.as_deref() { - if !table_exists(&self.pool, &normalized.profile_name, table_name).await? { - return Err(Status::not_found(format!( - "Table '{}' was not found in profile '{}'", - table_name, normalized.profile_name - ))); - } + if let Some(table_name) = normalized.table_name.clone() { + normalized.table_name = Some( + visible_table_name(&self.pool, &normalized.profile_name, &table_name) + .await? + .ok_or_else(|| Status::not_found(format!( + "Table '{}' was not found in profile '{}'", + table_name, normalized.profile_name + )))?, + ); } if !normalized.has_input() { @@ -266,8 +268,8 @@ impl SearcherService { &self, request: Request, ) -> Result, Status> { - let normalized = normalize_request(request.into_inner())?; - let table_name = normalized.table_name.as_deref().ok_or_else(|| { + let mut normalized = normalize_request(request.into_inner())?; + let requested_table_name = normalized.table_name.clone().ok_or_else(|| { Status::invalid_argument("table_name is required when counting search results") })?; if !normalized.has_input() { @@ -275,11 +277,17 @@ impl SearcherService { "counting search results requires text or a column constraint", )); } - if !profile_exists(&self.pool, &normalized.profile_name).await? - || !table_exists(&self.pool, &normalized.profile_name, table_name).await? - { + if !profile_exists(&self.pool, &normalized.profile_name).await? { return Err(Status::not_found("Search table was not found")); } + normalized.table_name = visible_table_name( + &self.pool, + &normalized.profile_name, + &requested_table_name, + ) + .await?; + let table_name = normalized.table_name.as_deref() + .ok_or_else(|| Status::not_found("Search table was not found"))?; let index_path = search_index_path( &common::search::search_index_root(), &normalized.profile_name, @@ -744,6 +752,25 @@ fn validate_identifier(value: &str, field_name: &str) -> Result<(), Status> { Ok(()) } +fn validate_table_name(value: &str) -> Result<(), Status> { + if value.is_empty() + || !value.chars().all(|character| { + character.is_ascii_alphanumeric() || matches!(character, '_' | '-') + }) + || !value + .chars() + .next() + .is_some_and(|character| character.is_ascii_alphanumeric()) + || !value + .chars() + .last() + .is_some_and(|character| character.is_ascii_alphanumeric()) + { + return Err(Status::invalid_argument("table_name contains invalid characters")); + } + Ok(()) +} + fn validate_search_column(value: &str) -> Result<(), Status> { if value.is_empty() { return Err(Status::invalid_argument( @@ -774,25 +801,26 @@ async fn profile_exists(pool: &PgPool, profile_name: &str) -> Result Result { - let exists = sqlx::query_scalar::<_, bool>( - r#" - SELECT EXISTS( - SELECT 1 - FROM table_definitions td - JOIN schemas s ON td.schema_id = s.id - WHERE td.table_name = $2 - AND td.deleted = FALSE - AND (s.name = $1 OR td.is_global = TRUE) - ) - "#, +async fn visible_table_name( + pool: &PgPool, + profile_name: &str, + table_name: &str, +) -> Result, Status> { + sqlx::query_scalar::<_, String>( + r#"SELECT td.table_name + FROM table_definitions td + JOIN schemas s ON td.schema_id = s.id + WHERE td.canonical_table_name = $2 + AND td.deleted = FALSE + AND (s.name = $1 OR td.is_global = TRUE) + ORDER BY td.is_global ASC + LIMIT 1"#, ) .bind(profile_name) - .bind(table_name) - .fetch_one(pool) + .bind(common::alias::canonical_table_name(table_name)) + .fetch_optional(pool) .await - .map_err(|e| Status::internal(format!("Table lookup failed: {}", e)))?; - Ok(exists) + .map_err(|e| Status::internal(format!("Table lookup failed: {}", e))) } async fn qualified_visible_table( @@ -800,23 +828,24 @@ async fn qualified_visible_table( profile_name: &str, table_name: &str, ) -> Result { - let storage_schema = sqlx::query_scalar::<_, String>( - r#"SELECT owner.name + let resolved = sqlx::query_as::<_, (String, String)>( + r#"SELECT owner.name, definition.table_name FROM table_definitions definition JOIN schemas owner ON owner.id = definition.schema_id - WHERE definition.table_name = $2 + WHERE definition.canonical_table_name = $2 AND definition.deleted = FALSE AND (owner.name = $1 OR definition.is_global = TRUE) ORDER BY definition.is_global ASC LIMIT 1"#, ) .bind(profile_name) - .bind(table_name) + .bind(common::alias::canonical_table_name(table_name)) .fetch_optional(pool) .await .map_err(|error| Status::internal(format!("Table storage lookup failed: {error}")))? .ok_or_else(|| Status::not_found(format!("Table '{table_name}' was not found")))?; - Ok(qualify_profile_table(&storage_schema, table_name)) + let (storage_schema, stored_table_name) = resolved; + Ok(qualify_profile_table(&storage_schema, &stored_table_name)) } fn normalize_request(req: SearchRequest) -> Result { @@ -828,7 +857,7 @@ fn normalize_request(req: SearchRequest) -> Result { - validate_identifier(table_name, "table_name")?; + validate_table_name(table_name)?; Some(table_name.to_string()) } _ => None, diff --git a/server b/server index dddd59fd..32cab246 160000 --- a/server +++ b/server @@ -1 +1 @@ -Subproject commit dddd59fd6167ec5a0126b26bcda6ddc48539cd99 +Subproject commit 32cab246fa68c29eae7759bdde7cfd567b24c6f5 diff --git a/web/locales/cs/main.ftl b/web/locales/cs/main.ftl index 1873094a..bb99cd90 100644 --- a/web/locales/cs/main.ftl +++ b/web/locales/cs/main.ftl @@ -853,6 +853,8 @@ error-identifier-underscore = { $label } nesmí začínat podtržítkem. error-identifier-number = { $label } nesmí začínat číslicí. error-identifier-too-long = { $label } nesmí být delší než { $limit } znaků. error-identifier-charset = { $label } může obsahovat jen malá písmena, číslice a podtržítko. +error-table-name-charset = { $label } může obsahovat jen ASCII písmena, číslice, podtržítka a pomlčky. +error-table-name-boundary = { $label } musí začínat a končit písmenem nebo číslicí. error-alias-charset = { $label } musí obsahovat písmeno, číslo, interpunkci nebo symbol mimo emoji a nesmí obsahovat řídicí znaky. error-alias-too-long = { $label } nesmí být delší než { $limit } bajtů UTF-8. error-identifier-reserved = { $label } používá vyhrazený název. diff --git a/web/locales/en/main.ftl b/web/locales/en/main.ftl index c1e72766..8757ba69 100644 --- a/web/locales/en/main.ftl +++ b/web/locales/en/main.ftl @@ -838,6 +838,8 @@ error-identifier-underscore = { $label } cannot start with an underscore. error-identifier-number = { $label } cannot start with a number. error-identifier-too-long = { $label } cannot be longer than { $limit } characters. error-identifier-charset = { $label } may only use lowercase letters, digits and underscores. +error-table-name-charset = { $label } may only use ASCII letters, digits, underscores and hyphens. +error-table-name-boundary = { $label } must start and end with a letter or number. error-alias-charset = { $label } must contain a letter, number, punctuation mark or non-emoji symbol, and cannot contain control characters. error-alias-too-long = { $label } cannot be longer than { $limit } UTF-8 bytes. error-identifier-reserved = { $label } uses a reserved name. diff --git a/web/locales/sk/main.ftl b/web/locales/sk/main.ftl index cd48b799..261162a3 100644 --- a/web/locales/sk/main.ftl +++ b/web/locales/sk/main.ftl @@ -851,6 +851,8 @@ error-identifier-underscore = { $label } nesmie začínať podčiarkovníkom. error-identifier-number = { $label } nesmie začínať číslom. error-identifier-too-long = { $label } nesmie byť dlhšie ako { $limit } znakov. error-identifier-charset = { $label } môže obsahovať len malé písmená, číslice a podčiarkovník. +error-table-name-charset = { $label } môže obsahovať len ASCII písmená, číslice, podčiarkovníky a pomlčky. +error-table-name-boundary = { $label } musí začínať a končiť písmenom alebo číslicou. error-alias-charset = { $label } musí obsahovať písmeno, číslo, interpunkciu alebo symbol mimo emoji a nesmie obsahovať riadiace znaky. error-alias-too-long = { $label } nesmie byť dlhšie ako { $limit } bajtov UTF-8. error-identifier-reserved = { $label } používa vyhradený názov. diff --git a/web/src/pages/add_table/draft.rs b/web/src/pages/add_table/draft.rs index 7c21546f..b9f47ac5 100644 --- a/web/src/pages/add_table/draft.rs +++ b/web/src/pages/add_table/draft.rs @@ -224,7 +224,10 @@ impl TableDraft { ) { self.relation_table_options = options .into_iter() - .filter(|option| option.name != self.table_name) + .filter(|option| { + common::alias::canonical_table_name(&option.name) + != common::alias::canonical_table_name(&self.table_name) + }) .collect(); self.relation_tables = self .relation_table_options @@ -399,7 +402,10 @@ impl TableDraft { && self .existing_profile_tables .iter() - .any(|name| name == &self.table_name) + .any(|name| { + common::alias::canonical_table_name(name) + == common::alias::canonical_table_name(&self.table_name) + }) } /// Position of `column` among the display columns, counting from 1. diff --git a/web/src/pages/add_table/loader.rs b/web/src/pages/add_table/loader.rs index a5f38f25..ab8d8bed 100644 --- a/web/src/pages/add_table/loader.rs +++ b/web/src/pages/add_table/loader.rs @@ -71,7 +71,7 @@ pub(crate) async fn load_page( // for, so it is told what that table is on every render rather than left to // read a copy that a `refresh` could have moved on from. draft.columns.global = draft.global; - draft.columns.table_name = draft.table_name.trim().to_ascii_lowercase(); + draft.columns.table_name = draft.table_name.trim().to_string(); let tree = definitions .get_profile_tree( diff --git a/web/src/pages/add_table/state.rs b/web/src/pages/add_table/state.rs index c8cdc0c6..a0845f92 100644 --- a/web/src/pages/add_table/state.rs +++ b/web/src/pages/add_table/state.rs @@ -140,7 +140,7 @@ impl BuilderForm { // takes no column that posts to one profile's books, and no link // may point at the table being created. global: self.global, - table_name: self.table_name.trim().to_ascii_lowercase(), + table_name: self.table_name.trim().to_string(), }; // Drop display columns whose column is gone, so a stale post cannot diff --git a/web/src/schema/mod.rs b/web/src/schema/mod.rs index 7771ab47..4c8281b3 100644 --- a/web/src/schema/mod.rs +++ b/web/src/schema/mod.rs @@ -579,7 +579,7 @@ impl ColumnDraft { return Ok(None); } if self.catalog.is_link(&column_type) { - let target = self.link_table_input.trim().to_ascii_lowercase(); + let target = self.link_table_input.trim(); return Ok((!target.is_empty()).then(|| format!("{column_type}({target})"))); } let Some(group) = self.pending_group() else { @@ -1209,8 +1209,28 @@ pub(crate) fn validate_table_name( locale: crate::i18n::Locale, value: &str, ) -> Option { - if let Some(error) = validate_identifier(locale, value, "label-table-name", true) { - return Some(error); + let label = crate::tr!(locale, "label-table-name"); + if value.is_empty() { + return Some(crate::tr!(locale, "error-identifier-empty", "label" => label)); + } + if value != value.trim() { + return Some(crate::tr!(locale, "error-identifier-whitespace", "label" => label)); + } + if !value.chars().all(|character| { + character.is_ascii_alphanumeric() || matches!(character, '_' | '-') + }) { + return Some(crate::tr!(locale, "error-table-name-charset", "label" => label)); + } + if !value + .chars() + .next() + .is_some_and(|character| character.is_ascii_alphanumeric()) + || !value + .chars() + .last() + .is_some_and(|character| character.is_ascii_alphanumeric()) + { + return Some(crate::tr!(locale, "error-table-name-boundary", "label" => label)); } if value.len() > MAX_TABLE_NAME_LENGTH { // The limit is arithmetic, not a literal: it moves when a system column @@ -1221,7 +1241,10 @@ pub(crate) fn validate_table_name( "limit" => MAX_TABLE_NAME_LENGTH as i64, )); } - if RESERVED_TABLE_NAMES.contains(&value) { + let canonical = common::alias::canonical_table_name(value); + if RESERVED_TABLE_NAMES.contains(&canonical.as_str()) + || crate::system_column::is_system_column(&canonical) + { return Some(crate::tr!( locale, "schema-err-table-name-reserved", @@ -2305,6 +2328,12 @@ pub(crate) mod tests { .unwrap_or_else(|| panic!("`{name}` should be reserved")); assert!(error.contains(name), "{error}"); } + assert_eq!(validate_table_name(crate::i18n::Locale::default(), "Custom_Exchange_Rates"), + validate_table_name(crate::i18n::Locale::default(), "custom_exchange_rates")); + assert_eq!(validate_table_name(crate::i18n::Locale::default(), "2026-Sales_Q4"), None); + for name in ["_sales", "sales_", "-sales", "sales-"] { + assert!(validate_table_name(crate::i18n::Locale::default(), name).is_some()); + } assert_eq!(validate_table_name(crate::i18n::Locale::default(), "invoice"), None); } }