From 1b7dbd9074116116e01ccddbd403f98170997b5a Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:24:13 +0800 Subject: [PATCH 1/7] fix(typeguard): resolve in the session database and keep the CONVERT source key Three defects on the typeguard and CONVERT paths: - CREATE TYPEGUARD read DatabaseId::DEFAULT while collections live under the session database; thread database_id through the seven handlers and validate_typeguard - CONVERT rebuilt the strict schema from the column list and dropped the source primary key, so every later insert failed "no resolved primary key"; mark the source key column and refuse a column list that omits it - a guard DEFAULT or VALUE naming another column became a strict column DEFAULT that evaluates with no row in scope; refuse the guard at CONVERT, naming the field and clause --- nodedb-sql/src/planner/defaults/compiled.rs | 55 +++++++++++++++++++ nodedb-sql/src/planner/defaults/mod.rs | 4 +- .../shared/ddl/neutral/convert/driver.rs | 43 +++++++++++++++ .../ddl/neutral/convert/typeguard_columns.rs | 32 ++++++++++- .../ddl/neutral/router/string_schema.rs | 27 +++++++-- .../shared/ddl/neutral/typeguard/handlers.rs | 31 +++++++---- .../shared/ddl/neutral/typeguard/validate.rs | 5 +- 7 files changed, 175 insertions(+), 22 deletions(-) diff --git a/nodedb-sql/src/planner/defaults/compiled.rs b/nodedb-sql/src/planner/defaults/compiled.rs index 9905e3e23..2568d7d89 100644 --- a/nodedb-sql/src/planner/defaults/compiled.rs +++ b/nodedb-sql/src/planner/defaults/compiled.rs @@ -186,6 +186,61 @@ pub fn validate_default_expr(expr: &str, column: &str) -> crate::Result<()> { CompiledDefault::declare(column, expr).map(|_| ()) } +/// Whether a declared value-producing expression references a row column. +/// +/// A column `DEFAULT` and a typeguard `DEFAULT`/`VALUE` are evaluated with no +/// row in scope, so an expression that names a column can never produce a +/// value there. Callers that carry such an expression into a column `DEFAULT` +/// (CONVERT's typeguard path) use this to refuse at declaration time instead +/// of failing the first insert with [`crate::SqlError::UnevaluableDefault`]. +/// +/// Classifies and parses through the same resolver gate a DEFAULT passes; +/// evaluates nothing. A second name list or expression walker is not written. +pub fn default_expr_references_columns(expr: &str) -> crate::Result { + let parsed = crate::parse_expr_string(expr)?; + Ok(expr_references_column(&parsed)) +} + +/// Recursive column-reference scan over a parsed DEFAULT expression. +fn expr_references_column(expr: &SqlExpr) -> bool { + match expr { + SqlExpr::Column { .. } => true, + SqlExpr::Literal(_) | SqlExpr::Wildcard | SqlExpr::Subquery(_) => false, + SqlExpr::BinaryOp { left, right, .. } => { + expr_references_column(left) || expr_references_column(right) + } + SqlExpr::UnaryOp { expr, .. } + | SqlExpr::IsNull { expr, .. } + | SqlExpr::Cast { expr, .. } => expr_references_column(expr), + SqlExpr::Function { args, .. } => args.iter().any(expr_references_column), + SqlExpr::Case { + operand, + when_then, + else_expr, + } => { + operand.as_deref().is_some_and(expr_references_column) + || when_then + .iter() + .any(|(w, t)| expr_references_column(w) || expr_references_column(t)) + || else_expr.as_deref().is_some_and(expr_references_column) + } + SqlExpr::InList { expr, list, .. } => { + expr_references_column(expr) || list.iter().any(expr_references_column) + } + SqlExpr::Between { + expr, low, high, .. + } => { + expr_references_column(expr) + || expr_references_column(low) + || expr_references_column(high) + } + SqlExpr::Like { expr, pattern, .. } => { + expr_references_column(expr) || expr_references_column(pattern) + } + SqlExpr::ArrayLiteral(items) => items.iter().any(expr_references_column), + } +} + /// Classify a DEFAULT into its compiled form. fn classify(column: &str, expr: &str) -> crate::Result { let upper = expr.trim().to_uppercase(); diff --git a/nodedb-sql/src/planner/defaults/mod.rs b/nodedb-sql/src/planner/defaults/mod.rs index 98b05a51b..a9755e854 100644 --- a/nodedb-sql/src/planner/defaults/mod.rs +++ b/nodedb-sql/src/planner/defaults/mod.rs @@ -26,5 +26,7 @@ mod compiled; mod convert; mod kind; -pub use compiled::{ColumnDefaults, CompiledDefault, validate_default_expr}; +pub use compiled::{ + ColumnDefaults, CompiledDefault, default_expr_references_columns, validate_default_expr, +}; pub use convert::default_value_to_sql; diff --git a/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs b/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs index 66fb8f94d..fbacb7335 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs @@ -61,6 +61,49 @@ pub async fn convert_collection( _ => None, }; + // Preserve the source collection's declared identity through the + // conversion. Without this, a schemaless source declared `id TEXT PRIMARY + // KEY` converts to a strict schema whose columns carry no primary key, and + // every insert after the conversion fails `no resolved primary key`. A + // column list that omits the source key is refused rather than silently + // minting new row identities. + let mut columns = columns; + if let Some(cols) = columns.as_mut() { + let source_pk = match &coll.collection_type { + nodedb_types::CollectionType::Document(nodedb_types::DocumentMode::Strict(schema)) => { + schema + .columns + .iter() + .find(|c| c.primary_key) + .map(|c| c.name.clone()) + } + nodedb_types::CollectionType::KeyValue(config) => config + .schema + .columns + .iter() + .find(|c| c.primary_key) + .map(|c| c.name.clone()), + nodedb_types::CollectionType::Document(nodedb_types::DocumentMode::Schemaless) => { + coll.declared_primary_key.clone() + } + nodedb_types::CollectionType::Columnar(_) => None, + }; + if let Some(ref pk) = source_pk { + match cols.iter_mut().find(|c| &c.name == pk) { + Some(col) => col.primary_key = true, + None => { + return Err(err( + "42601", + format!( + "converted schema must keep the source primary key column \ + '{pk}'; it is absent from the column list" + ), + )); + } + } + } + } + let schema_json_for_dp = if let Some(ref cols) = columns { sonic_rs::to_string(cols).map_err(|e| err("XX000", format!("schema serialization: {e}")))? } else { diff --git a/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs b/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs index 0bc1749cc..11d6e5196 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs @@ -49,8 +49,38 @@ pub(super) fn typeguards_to_column_defs( }; // A guard carries either DEFAULT or VALUE, never both. Strict schema // has one materialization slot, so both land on the column `DEFAULT`. - if let Some(expr) = guard.default_expr.clone().or(guard.value_expr.clone()) { + let carried = guard + .default_expr + .clone() + .map(|expr| ("DEFAULT", expr)) + .or(guard.value_expr.clone().map(|expr| ("VALUE", expr))); + if let Some((clause, expr)) = carried { validate_column_default(&col.name, &expr)?; + // A guard VALUE is evaluated per row against the document; a + // strict-schema column DEFAULT is evaluated with no row in scope. + // Carrying a column-referencing expression over would accept the + // CONVERT and fail every insert with `UnevaluableDefault`, so it + // is refused here, naming the clause and the field. + let references_column = nodedb_sql::planner::defaults::default_expr_references_columns( + &expr, + ) + .map_err(|e| { + err( + "42601", + format!("field '{}': {clause} is invalid: {e}", guard.field), + ) + })?; + if references_column { + return Err(err( + "42601", + format!( + "field '{}': {clause} expression '{expr}' references another column; \ + a strict-schema column DEFAULT is evaluated with no row in scope. \ + Give a constant expression, or keep the collection schemaless", + guard.field + ), + )); + } col.default = Some(expr); } columns.push(col); diff --git a/nodedb/src/control/server/shared/ddl/neutral/router/string_schema.rs b/nodedb/src/control/server/shared/ddl/neutral/router/string_schema.rs index 53d24cb8b..d634ca80d 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/router/string_schema.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/router/string_schema.rs @@ -138,22 +138,37 @@ pub(super) async fn try_string( // before the parse gate). Replicate that exactly here, before the parse // gate, so the prefix recognition and syntax messages stay byte-identical. if upper.starts_with("CREATE TYPEGUARD ") || upper.starts_with("CREATE OR REPLACE TYPEGUARD ") { - return Some(typeguard::create_typeguard(state, identity, sql)); + return Some(typeguard::create_typeguard( + state, + identity, + database_id, + sql, + )); } if upper.starts_with("ALTER TYPEGUARD ") { - return Some(typeguard::alter_typeguard(state, identity, sql)); + return Some(typeguard::alter_typeguard( + state, + identity, + database_id, + sql, + )); } if upper.starts_with("DROP TYPEGUARD ") { - return Some(typeguard::drop_typeguard(state, identity, sql)); + return Some(typeguard::drop_typeguard(state, identity, database_id, sql)); } if upper.starts_with("VALIDATE TYPEGUARD ON ") { - return Some(typeguard::validate_typeguard(state, identity, sql).await); + return Some(typeguard::validate_typeguard(state, identity, database_id, sql).await); } if upper.starts_with("SHOW TYPEGUARD ON ") { - return Some(typeguard::show_typeguard(state, identity, sql)); + return Some(typeguard::show_typeguard(state, identity, database_id, sql)); } if upper == "SHOW TYPEGUARDS" || upper.starts_with("SHOW TYPEGUARDS") { - return Some(typeguard::show_typeguards(state, identity, sql)); + return Some(typeguard::show_typeguards( + state, + identity, + database_id, + sql, + )); } None diff --git a/nodedb/src/control/server/shared/ddl/neutral/typeguard/handlers.rs b/nodedb/src/control/server/shared/ddl/neutral/typeguard/handlers.rs index 19ab0c828..3c18678e2 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/typeguard/handlers.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/typeguard/handlers.rs @@ -58,6 +58,7 @@ fn status(command: &str) -> Vec { pub fn create_typeguard( state: &SharedState, identity: &AuthenticatedIdentity, + database_id: DatabaseId, sql: &str, ) -> Result, DdlError> { let or_replace = find_ascii_case_insensitive(sql, "OR REPLACE").is_some(); @@ -77,7 +78,7 @@ pub fn create_typeguard( let tenant_id = identity.tenant_id.as_u64(); let mut coll = catalog - .get_collection(DatabaseId::DEFAULT, tenant_id, &coll_name) + .get_collection(database_id, tenant_id, &coll_name) .map_err(|e| err("XX000", &e.to_string()))? .ok_or_else(|| err("42P01", &format!("collection '{coll_name}' not found")))?; @@ -100,7 +101,7 @@ pub fn create_typeguard( } coll.type_guards = guards; - persist_collection_replicated(state, DatabaseId::DEFAULT, &coll) + persist_collection_replicated(state, database_id, &coll) .map_err(|e| err("XX000", &e.to_string()))?; state.schema_version.bump(); @@ -114,6 +115,7 @@ pub fn create_typeguard( pub fn alter_typeguard_add( state: &SharedState, identity: &AuthenticatedIdentity, + database_id: DatabaseId, sql: &str, ) -> Result, DdlError> { let coll_name = extract_collection_name(sql)?; @@ -128,7 +130,7 @@ pub fn alter_typeguard_add( let tenant_id = identity.tenant_id.as_u64(); let mut coll = catalog - .get_collection(DatabaseId::DEFAULT, tenant_id, &coll_name) + .get_collection(database_id, tenant_id, &coll_name) .map_err(|e| err("XX000", &e.to_string()))? .ok_or_else(|| err("42P01", &format!("collection '{coll_name}' not found")))?; @@ -152,7 +154,7 @@ pub fn alter_typeguard_add( } coll.type_guards.push(guard); - persist_collection_replicated(state, DatabaseId::DEFAULT, &coll) + persist_collection_replicated(state, database_id, &coll) .map_err(|e| err("XX000", &e.to_string()))?; state.schema_version.bump(); @@ -164,6 +166,7 @@ pub fn alter_typeguard_add( pub fn alter_typeguard_drop( state: &SharedState, identity: &AuthenticatedIdentity, + database_id: DatabaseId, sql: &str, ) -> Result, DdlError> { let coll_name = extract_collection_name(sql)?; @@ -180,7 +183,7 @@ pub fn alter_typeguard_drop( let tenant_id = identity.tenant_id.as_u64(); let mut coll = catalog - .get_collection(DatabaseId::DEFAULT, tenant_id, &coll_name) + .get_collection(database_id, tenant_id, &coll_name) .map_err(|e| err("XX000", &e.to_string()))? .ok_or_else(|| err("42P01", &format!("collection '{coll_name}' not found")))?; @@ -194,7 +197,7 @@ pub fn alter_typeguard_drop( )); } - persist_collection_replicated(state, DatabaseId::DEFAULT, &coll) + persist_collection_replicated(state, database_id, &coll) .map_err(|e| err("XX000", &e.to_string()))?; state.schema_version.bump(); @@ -206,13 +209,14 @@ pub fn alter_typeguard_drop( pub fn alter_typeguard( state: &SharedState, identity: &AuthenticatedIdentity, + database_id: DatabaseId, sql: &str, ) -> Result, DdlError> { let upper = sql.to_uppercase(); if upper.contains(" ADD ") { - alter_typeguard_add(state, identity, sql) + alter_typeguard_add(state, identity, database_id, sql) } else if upper.contains(" DROP ") { - alter_typeguard_drop(state, identity, sql) + alter_typeguard_drop(state, identity, database_id, sql) } else { Err(err( "42601", @@ -227,6 +231,7 @@ pub fn alter_typeguard( pub fn drop_typeguard( state: &SharedState, identity: &AuthenticatedIdentity, + database_id: DatabaseId, sql: &str, ) -> Result, DdlError> { let upper = sql.to_uppercase(); @@ -238,7 +243,7 @@ pub fn drop_typeguard( let tenant_id = identity.tenant_id.as_u64(); let mut coll = catalog - .get_collection(DatabaseId::DEFAULT, tenant_id, &coll_name) + .get_collection(database_id, tenant_id, &coll_name) .map_err(|e| err("XX000", &e.to_string()))? .ok_or_else(|| err("42P01", &format!("collection '{coll_name}' not found")))?; @@ -253,7 +258,7 @@ pub fn drop_typeguard( } coll.type_guards.clear(); - persist_collection_replicated(state, DatabaseId::DEFAULT, &coll) + persist_collection_replicated(state, database_id, &coll) .map_err(|e| err("XX000", &e.to_string()))?; state.schema_version.bump(); @@ -267,6 +272,7 @@ pub fn drop_typeguard( pub fn show_typeguard( state: &SharedState, identity: &AuthenticatedIdentity, + database_id: DatabaseId, sql: &str, ) -> Result, DdlError> { let coll_name = extract_collection_name(sql)?; @@ -275,7 +281,7 @@ pub fn show_typeguard( let tenant_id = identity.tenant_id.as_u64(); let coll = catalog - .get_collection(DatabaseId::DEFAULT, tenant_id, &coll_name) + .get_collection(database_id, tenant_id, &coll_name) .map_err(|e| err("XX000", &e.to_string()))? .ok_or_else(|| err("42P01", &format!("collection '{coll_name}' not found")))?; @@ -316,13 +322,14 @@ pub fn show_typeguard( pub fn show_typeguards( state: &SharedState, identity: &AuthenticatedIdentity, + database_id: DatabaseId, _sql: &str, ) -> Result, DdlError> { let catalog = state.credentials.catalog(); let tenant_id = identity.tenant_id.as_u64(); let collections = catalog - .load_collections_for_tenant(DatabaseId::DEFAULT, tenant_id) + .load_collections_for_tenant(database_id, tenant_id) .map_err(|e| err("XX000", &e.to_string()))?; let columns = vec!["collection".to_string(), "fields".to_string()]; diff --git a/nodedb/src/control/server/shared/ddl/neutral/typeguard/validate.rs b/nodedb/src/control/server/shared/ddl/neutral/typeguard/validate.rs index 141675359..43237a971 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/typeguard/validate.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/typeguard/validate.rs @@ -30,6 +30,7 @@ use super::super::super::result::{DdlError, DdlResult}; pub async fn validate_typeguard( state: &SharedState, identity: &AuthenticatedIdentity, + database_id: DatabaseId, sql: &str, ) -> Result, DdlError> { let coll_name = super::parse::extract_collection_name(sql)?; @@ -38,7 +39,7 @@ pub async fn validate_typeguard( let catalog = state.credentials.catalog(); let coll = catalog - .get_collection(DatabaseId::DEFAULT, tenant_id.as_u64(), &coll_name) + .get_collection(database_id, tenant_id.as_u64(), &coll_name) .map_err(|e| super::parse::err("XX000", &format!("catalog error: {e}")))? .ok_or_else(|| { super::parse::err("42P01", &format!("collection '{coll_name}' not found")) @@ -70,7 +71,7 @@ pub async fn validate_typeguard( state, identity, &scan_sql, - DatabaseId::DEFAULT, + database_id, ) .await .map_err(|error| super::parse::err(&error.sqlstate, &error.message))?; From 780176facd1410677a30143cb54c89e2b6280e1d Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:49:30 +0800 Subject: [PATCH 2/7] fix(convert): refuse a cross-column guard on the explicit-list path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The explicit column list defines the schema and carries no guard onto it. A guard the list covers and that names another column has no column DEFAULT equivalent: evaluated with no row in scope it fails every insert, and dropping it loses the guard's meaning silently. Refuse it at CONVERT, naming field and clause — the shape the filed reproduction takes. Coverage: the filed reproduction as a refusal, CONVERT keeping the source primary key, a list that omits it refused, and a guard declared in a session database (declaration resolves there). --- .../shared/ddl/neutral/convert/driver.rs | 40 ++++++++++ .../wire/cases/sql_convert_column_defs.rs | 80 +++++++++++++++++++ .../wire/cases/sql_typeguard_default_gate.rs | 31 +++++++ 3 files changed, 151 insertions(+) diff --git a/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs b/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs index fbacb7335..48490f2c9 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs @@ -47,6 +47,46 @@ pub async fn convert_collection( let columns: Option> = match target_type.as_str() { "document_strict" | "kv" => { let cols = if let Some(cols) = explicit_columns { + // The list defines the schema; no guard is carried onto it. A + // guard the list covers and that names another column has no + // column DEFAULT equivalent — evaluated with no row in scope it + // would fail every insert, and dropping it would lose the + // guard's meaning silently. Refuse it, naming field and clause. + for guard in &coll.type_guards { + if !cols.iter().any(|c| c.name == guard.field) { + continue; + } + let carried = guard + .default_expr + .as_deref() + .map(|e| ("DEFAULT", e)) + .or(guard.value_expr.as_deref().map(|e| ("VALUE", e))); + if let Some((clause, expr)) = carried { + let references_column = + nodedb_sql::planner::defaults::default_expr_references_columns(expr) + .map_err(|e| { + err( + "42601", + format!( + "field '{}': {clause} is invalid: {e}", + guard.field + ), + ) + })?; + if references_column { + return Err(err( + "42601", + format!( + "field '{}': {clause} expression '{expr}' references another \ + column; a strict-schema column DEFAULT is evaluated with no \ + row in scope. Give a constant expression, or keep the \ + collection schemaless", + guard.field + ), + )); + } + } + } cols } else if !coll.type_guards.is_empty() { typeguards_to_column_defs(&coll.type_guards)? diff --git a/nodedb/tests/wire/cases/sql_convert_column_defs.rs b/nodedb/tests/wire/cases/sql_convert_column_defs.rs index 9844b0c86..e6dc8fe47 100644 --- a/nodedb/tests/wire/cases/sql_convert_column_defs.rs +++ b/nodedb/tests/wire/cases/sql_convert_column_defs.rs @@ -86,3 +86,83 @@ async fn convert_refuses_a_default_naming_an_unknown_function() { ) .await; } + +/// CONVERT rebuilt the strict schema from the column list alone and dropped the +/// source primary key, so every insert after the conversion failed with no +/// resolved primary key. The converted schema keeps the key column. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn convert_keeps_the_source_primary_key() { + let server = TestServer::start().await; + + server + .exec("CREATE COLLECTION conv_key (id TEXT PRIMARY KEY, v TEXT)") + .await + .unwrap(); + server + .exec("INSERT INTO conv_key (id, v) VALUES ('k1', 'a')") + .await + .unwrap(); + + server + .exec("CONVERT COLLECTION conv_key TO document_strict (id TEXT, v TEXT)") + .await + .unwrap(); + + server + .exec("INSERT INTO conv_key (id, v) VALUES ('k2', 'b')") + .await + .expect("an insert after CONVERT must resolve a primary key"); + + let rows = server + .query_text("SELECT id FROM conv_key ORDER BY id") + .await + .unwrap(); + assert_eq!( + rows, + vec!["k1".to_string(), "k2".to_string()], + "both rows must be addressable: {rows:?}" + ); +} + +/// A column list that omits the source key is refused: the converted schema +/// would carry no primary key and every later insert would be unresolvable. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn convert_refuses_a_column_list_that_omits_the_source_key() { + let server = TestServer::start().await; + + server + .exec("CREATE COLLECTION conv_nokey (id TEXT PRIMARY KEY, v TEXT)") + .await + .unwrap(); + + server + .expect_error( + "CONVERT COLLECTION conv_nokey TO document_strict (v TEXT)", + "42601", + ) + .await; +} + +/// A guard that names another column is a per-row expression. Carried onto a +/// strict column it becomes a DEFAULT evaluated with no row in scope, which +/// fails every insert. CONVERT refuses it, naming the field and the clause. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn convert_refuses_a_guard_that_references_another_column() { + let server = TestServer::start().await; + + server + .exec("CREATE COLLECTION conv_guard (id TEXT PRIMARY KEY, status TEXT, lowered TEXT)") + .await + .unwrap(); + server + .exec("CREATE TYPEGUARD ON conv_guard (lowered STRING VALUE LOWER(status))") + .await + .unwrap(); + + let convert = "CONVERT COLLECTION conv_guard TO document_strict \ + (id TEXT, status TEXT, lowered TEXT)"; + server.expect_error(convert, "lowered").await; + server + .expect_error(convert, "references another column") + .await; +} diff --git a/nodedb/tests/wire/cases/sql_typeguard_default_gate.rs b/nodedb/tests/wire/cases/sql_typeguard_default_gate.rs index 7f621680c..4b9d81ecd 100644 --- a/nodedb/tests/wire/cases/sql_typeguard_default_gate.rs +++ b/nodedb/tests/wire/cases/sql_typeguard_default_gate.rs @@ -98,3 +98,34 @@ async fn typeguard_evaluable_defaults_stay_accepted() { "DEFAULT must still inject: {stored:?}" ); } + +/// `CREATE TYPEGUARD` resolves its target in the session database. The +/// handlers read the default database while collections live under the session +/// one, so a guard declared in another database answered `42P01` for a +/// collection that exists. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn typeguard_declares_in_the_session_database() { + let server = TestServer::start().await; + + server.exec("CREATE DATABASE tg_session").await.unwrap(); + server.exec("USE DATABASE tg_session").await.unwrap(); + server + .exec("CREATE COLLECTION tg_here (id TEXT PRIMARY KEY, v TEXT)") + .await + .unwrap(); + + server + .exec("CREATE TYPEGUARD ON tg_here (v STRING REQUIRED)") + .await + .expect("the guard must declare in the session database"); + + let rows = server + .query_text("SHOW TYPEGUARD ON tg_here") + .await + .unwrap(); + assert_eq!( + rows, + vec!["v".to_string()], + "the guard must be stored against the session database: {rows:?}" + ); +} From 879b1329f43cf4f7482454547e8a69229005be26 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:19:33 +0800 Subject: [PATCH 3/7] fix(ddl): refuse a column DEFAULT that names another column at the one gate The reference check lived at two CONVERT call sites, so CREATE COLLECTION and a CONVERT column list still accepted a cross-column DEFAULT and failed at the first insert with UnevaluableDefault. The check moves into the gate every producer calls. - validate_constant_clause_expr: classify first, exactly as the default classifier does, so a generator (UUID_V7, gen_uuid_v7()) or a literal is never read as a column reference - CREATE COLLECTION, a CONVERT column list, a CONVERT typeguard carry, and the explicit-list guard path all pass through it; the two local copies are gone --- nodedb-sql/src/planner/defaults/compiled.rs | 25 +++++++++++++-- .../shared/ddl/neutral/column_default.rs | 29 ++++++++++++++++- .../shared/ddl/neutral/convert/driver.rs | 28 +++------------- .../ddl/neutral/convert/typeguard_columns.rs | 31 +++--------------- .../wire/cases/sql_default_expressions.rs | 32 +++++++++++++++++++ 5 files changed, 93 insertions(+), 52 deletions(-) diff --git a/nodedb-sql/src/planner/defaults/compiled.rs b/nodedb-sql/src/planner/defaults/compiled.rs index 2568d7d89..092efaba2 100644 --- a/nodedb-sql/src/planner/defaults/compiled.rs +++ b/nodedb-sql/src/planner/defaults/compiled.rs @@ -194,9 +194,16 @@ pub fn validate_default_expr(expr: &str, column: &str) -> crate::Result<()> { /// (CONVERT's typeguard path) use this to refuse at declaration time instead /// of failing the first insert with [`crate::SqlError::UnevaluableDefault`]. /// -/// Classifies and parses through the same resolver gate a DEFAULT passes; -/// evaluates nothing. A second name list or expression walker is not written. +/// The classification mirrors [`classify`]: a generator (`UUID_V7`, +/// `gen_uuid_v7()`) or a literal/parametric form carries no expression at all, +/// so only the `Expr` branch is parsed — and parsed through the same resolver +/// gate a DEFAULT passes. A second name list or expression walker is not +/// written. pub fn default_expr_references_columns(expr: &str) -> crate::Result { + let upper = expr.trim().to_uppercase(); + if keyword_generator(&upper).is_some() || parametric_or_literal(expr, &upper)?.is_some() { + return Ok(false); + } let parsed = crate::parse_expr_string(expr)?; Ok(expr_references_column(&parsed)) } @@ -409,4 +416,18 @@ mod tests { .expect_err("unknown function refused"); assert!(matches!(error, SqlError::UnevaluableDefault { .. })); } + + /// A generator or a literal is not a column reference. Only the `Expr` + /// branch the classifier parses can carry one. + #[test] + fn only_the_expression_branch_can_reference_a_column() { + for constant in ["UUID_V7", "uuid_v7()", "gen_uuid_v7()", "'active'", "42"] { + assert!( + !default_expr_references_columns(constant).unwrap(), + "{constant} is a constant form" + ); + } + assert!(default_expr_references_columns("LOWER(status)").unwrap()); + assert!(default_expr_references_columns("status || '-x'").unwrap()); + } } diff --git a/nodedb/src/control/server/shared/ddl/neutral/column_default.rs b/nodedb/src/control/server/shared/ddl/neutral/column_default.rs index 390409a66..935ac016f 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/column_default.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/column_default.rs @@ -38,7 +38,34 @@ pub(super) fn validate_column_defaults(columns: &[(String, String)]) -> Result<( /// Refuse one declared column `DEFAULT` the server cannot evaluate. pub(super) fn validate_column_default(column: &str, expr: &str) -> Result<(), DdlError> { - validate_clause_expr("DEFAULT", column, expr) + validate_constant_clause_expr("DEFAULT", column, expr) +} + +/// Refuse a value-producing clause whose value must be constant. +/// +/// A column `DEFAULT` is const-folded once, with no row in scope, so an +/// expression that names another column can never produce a value there. +/// Refusing it at this gate — the one every producer of a column `DEFAULT` +/// calls — keeps the refusal at the declaration instead of the first insert's +/// `UnevaluableDefault`. +pub(super) fn validate_constant_clause_expr( + clause: &str, + owner: &str, + expr: &str, +) -> Result<(), DdlError> { + validate_clause_expr(clause, owner, expr)?; + let references_column = nodedb_sql::planner::defaults::default_expr_references_columns(expr) + .map_err(|error| clause_error(clause, owner, &error))?; + if references_column { + return Err(DdlError::new( + sqlstate::SYNTAX_ERROR, + format!( + "{clause} for '{owner}' references another column; it is evaluated with no row \ + in scope, so give a constant expression" + ), + )); + } + Ok(()) } /// Refuse one declared value-producing clause the server cannot evaluate. diff --git a/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs b/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs index 48490f2c9..2a9309d92 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/convert/driver.rs @@ -21,6 +21,7 @@ use crate::control::state::SharedState; use nodedb_physical::physical_plan::MetaOp; use super::super::super::result::{DdlError, DdlResult}; +use super::super::column_default::validate_constant_clause_expr; use super::column_defs::parse_convert_sql; use super::support::err; use super::typeguard_columns::typeguards_to_column_defs; @@ -62,29 +63,10 @@ pub async fn convert_collection( .map(|e| ("DEFAULT", e)) .or(guard.value_expr.as_deref().map(|e| ("VALUE", e))); if let Some((clause, expr)) = carried { - let references_column = - nodedb_sql::planner::defaults::default_expr_references_columns(expr) - .map_err(|e| { - err( - "42601", - format!( - "field '{}': {clause} is invalid: {e}", - guard.field - ), - ) - })?; - if references_column { - return Err(err( - "42601", - format!( - "field '{}': {clause} expression '{expr}' references another \ - column; a strict-schema column DEFAULT is evaluated with no \ - row in scope. Give a constant expression, or keep the \ - collection schemaless", - guard.field - ), - )); - } + // The one gate refuses an unregistered function name + // and a column-referencing expression alike, naming + // the field and the clause the author wrote. + validate_constant_clause_expr(clause, &guard.field, expr)?; } } cols diff --git a/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs b/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs index 11d6e5196..d38941938 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs @@ -54,33 +54,12 @@ pub(super) fn typeguards_to_column_defs( .clone() .map(|expr| ("DEFAULT", expr)) .or(guard.value_expr.clone().map(|expr| ("VALUE", expr))); - if let Some((clause, expr)) = carried { + if let Some((_clause, expr)) = carried { + // The one gate refuses an unregistered function name and a + // column-referencing expression alike, naming the field. A guard + // VALUE evaluates per row against the document; the column DEFAULT + // it becomes does not. validate_column_default(&col.name, &expr)?; - // A guard VALUE is evaluated per row against the document; a - // strict-schema column DEFAULT is evaluated with no row in scope. - // Carrying a column-referencing expression over would accept the - // CONVERT and fail every insert with `UnevaluableDefault`, so it - // is refused here, naming the clause and the field. - let references_column = nodedb_sql::planner::defaults::default_expr_references_columns( - &expr, - ) - .map_err(|e| { - err( - "42601", - format!("field '{}': {clause} is invalid: {e}", guard.field), - ) - })?; - if references_column { - return Err(err( - "42601", - format!( - "field '{}': {clause} expression '{expr}' references another column; \ - a strict-schema column DEFAULT is evaluated with no row in scope. \ - Give a constant expression, or keep the collection schemaless", - guard.field - ), - )); - } col.default = Some(expr); } columns.push(col); diff --git a/nodedb/tests/wire/cases/sql_default_expressions.rs b/nodedb/tests/wire/cases/sql_default_expressions.rs index cee4bbd03..6deec7c22 100644 --- a/nodedb/tests/wire/cases/sql_default_expressions.rs +++ b/nodedb/tests/wire/cases/sql_default_expressions.rs @@ -441,3 +441,35 @@ fn assert_not_null(row: &str, label: &str) { "{label}: expected a value, got `{row}`" ); } + +/// A column `DEFAULT` is const-folded once, with no row in scope: an +/// expression that names another column can never produce a value. It is +/// refused at the declaration, not at the first insert's `UnevaluableDefault`. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_column_default_that_names_another_column_is_refused() { + let server = TestServer::start().await; + + server + .expect_error( + "CREATE COLLECTION def_selfref (\ + id TEXT PRIMARY KEY, \ + status TEXT, \ + lowered TEXT DEFAULT LOWER(status)) \ + WITH (engine='document_strict')", + "references another column", + ) + .await; + + // The same rule reaches a CONVERT column list. + server + .exec("CREATE COLLECTION def_selfref_conv") + .await + .unwrap(); + server + .expect_error( + "CONVERT COLLECTION def_selfref_conv TO document_strict \ + (id TEXT PRIMARY KEY, status TEXT, lowered TEXT DEFAULT LOWER(status))", + "references another column", + ) + .await; +} From 11a3c7b266c84545e856b91da1d54115370ede43 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Thu, 17 Sep 2026 02:23:30 +0800 Subject: [PATCH 4/7] fix(ddl): gate an ALTER ADD COLUMN DEFAULT like a CREATE column DEFAULT ALTER TABLE ADD COLUMN accepted a DEFAULT that names another column; the first insert failed with UnevaluableDefault. The declared definition passes the same gate CREATE and CONVERT columns pass. --- .../ddl/neutral/collection/alter/add_column.rs | 7 +++++++ nodedb/tests/wire/cases/sql_default_expressions.rs | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs b/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs index 9a641a494..10ad615b2 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs @@ -32,6 +32,13 @@ pub(super) async fn alter_table_add_column( let column = parse_origin_column_def(col_def_str).map_err(|e| err("42601", e.to_string()))?; let column_name = column.name.clone(); + // The declared DEFAULT passes the same gate a CREATE column passes: an + // unregistered function name, or an expression that names another column, + // is refused at the declaration rather than at the first insert. + super::super::super::column_default::validate_column_defaults(&[( + column_name.clone(), + col_def_str.to_string(), + )])?; // The declared type as written, e.g. `SMALLINT` from `age SMALLINT NOT // NULL`. `ColumnDef::column_type` cannot supply this: it has one `Int64` // variant for every integer width. Falls back to the resolved type's own diff --git a/nodedb/tests/wire/cases/sql_default_expressions.rs b/nodedb/tests/wire/cases/sql_default_expressions.rs index 6deec7c22..72501d127 100644 --- a/nodedb/tests/wire/cases/sql_default_expressions.rs +++ b/nodedb/tests/wire/cases/sql_default_expressions.rs @@ -472,4 +472,16 @@ async fn a_column_default_that_names_another_column_is_refused() { "references another column", ) .await; + + // And ALTER ... ADD COLUMN. + server + .exec("CREATE COLLECTION def_selfref_alter (id TEXT PRIMARY KEY, status TEXT)") + .await + .unwrap(); + server + .expect_error( + "ALTER TABLE def_selfref_alter ADD COLUMN lowered TEXT DEFAULT LOWER(status)", + "references another column", + ) + .await; } From 82ea86a916ba1c9076240143674191cd592dc79a Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Thu, 17 Sep 2026 04:25:44 +0800 Subject: [PATCH 5/7] fix(ddl): name the clause the author wrote on the carried guard A guard VALUE carried onto a column DEFAULT reported DEFAULT; the clause is VALUE. The carry path passes the clause through the one gate, so the refusal names what the author wrote. Same refusal either way; only the wording changes. --- .../shared/ddl/neutral/convert/typeguard_columns.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs b/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs index d38941938..e3ec2f498 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/convert/typeguard_columns.rs @@ -6,7 +6,7 @@ //! reads the collection's active typeguards instead. use super::super::super::result::DdlError; -use super::super::column_default::validate_column_default; +use super::super::column_default::validate_constant_clause_expr; use super::support::err; use super::type_map::typeguard_type_to_column_type; @@ -54,12 +54,12 @@ pub(super) fn typeguards_to_column_defs( .clone() .map(|expr| ("DEFAULT", expr)) .or(guard.value_expr.clone().map(|expr| ("VALUE", expr))); - if let Some((_clause, expr)) = carried { + if let Some((clause, expr)) = carried { // The one gate refuses an unregistered function name and a - // column-referencing expression alike, naming the field. A guard - // VALUE evaluates per row against the document; the column DEFAULT - // it becomes does not. - validate_column_default(&col.name, &expr)?; + // column-referencing expression alike, naming the field and the + // clause the author wrote. A guard VALUE evaluates per row against + // the document; the column DEFAULT it becomes does not. + validate_constant_clause_expr(clause, &col.name, &expr)?; col.default = Some(expr); } columns.push(col); From b9bc243fb498a7a6940b822b210bdc57691725f4 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Thu, 17 Sep 2026 07:04:01 +0800 Subject: [PATCH 6/7] fix(ddl): read an ALTER default from the parsed definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate passed the whole definition text as type text, and the type parser finds the clause by substring — a column named is_default was read as a DEFAULT clause and refused. The gate now reads the parsed default, and the parser's clause search requires the keyword to start a token, so a name that contains the word is never the clause. --- .../neutral/collection/alter/add_column.rs | 12 ++++---- .../shared/ddl/neutral/collection/helpers.rs | 4 ++- .../wire/cases/sql_default_expressions.rs | 28 +++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs b/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs index 10ad615b2..ddc61ee02 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs @@ -34,11 +34,13 @@ pub(super) async fn alter_table_add_column( let column_name = column.name.clone(); // The declared DEFAULT passes the same gate a CREATE column passes: an // unregistered function name, or an expression that names another column, - // is refused at the declaration rather than at the first insert. - super::super::super::column_default::validate_column_defaults(&[( - column_name.clone(), - col_def_str.to_string(), - )])?; + // is refused at the declaration rather than at the first insert. The gate + // reads the parsed default text, never the whole definition: the type + // parser finds the clause by substring, so a column name that contains the + // word `default` would otherwise be read as that clause. + if let Some(default_expr) = column.default.as_deref() { + super::super::super::column_default::validate_column_default(&column_name, default_expr)?; + } // The declared type as written, e.g. `SMALLINT` from `age SMALLINT NOT // NULL`. `ColumnDef::column_type` cannot supply this: it has one `Int64` // variant for every integer width. Falls back to the resolved type's own diff --git a/nodedb/src/control/server/shared/ddl/neutral/collection/helpers.rs b/nodedb/src/control/server/shared/ddl/neutral/collection/helpers.rs index 689b99fde..b4a45a043 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/collection/helpers.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/collection/helpers.rs @@ -59,7 +59,9 @@ pub(crate) fn parse_origin_column_def(s: &str) -> crate::Result Date: Thu, 17 Sep 2026 07:04:22 +0800 Subject: [PATCH 7/7] style: rustfmt --- .../control/server/shared/ddl/neutral/collection/helpers.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nodedb/src/control/server/shared/ddl/neutral/collection/helpers.rs b/nodedb/src/control/server/shared/ddl/neutral/collection/helpers.rs index b4a45a043..0ae8ce3a9 100644 --- a/nodedb/src/control/server/shared/ddl/neutral/collection/helpers.rs +++ b/nodedb/src/control/server/shared/ddl/neutral/collection/helpers.rs @@ -59,9 +59,7 @@ pub(crate) fn parse_origin_column_def(s: &str) -> crate::Result