diff --git a/nodedb-sql/src/planner/defaults/compiled.rs b/nodedb-sql/src/planner/defaults/compiled.rs index 69e25e460..ff1247267 100644 --- a/nodedb-sql/src/planner/defaults/compiled.rs +++ b/nodedb-sql/src/planner/defaults/compiled.rs @@ -199,6 +199,68 @@ 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`]. +/// +/// 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)) +} + +/// 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(); @@ -367,4 +429,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-sql/src/planner/defaults/mod.rs b/nodedb-sql/src/planner/defaults/mod.rs index 2e19ca84c..1cea20d50 100644 --- a/nodedb-sql/src/planner/defaults/mod.rs +++ b/nodedb-sql/src/planner/defaults/mod.rs @@ -23,5 +23,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/collection/alter/add_column.rs b/nodedb/src/control/server/shared/ddl/neutral/collection/alter/add_column.rs index 6976f6749..8d54b29d0 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 @@ -55,8 +55,11 @@ pub(super) async fn alter_table_add_column( ), )); } - // A DEFAULT passes the same gate `CREATE` applies: evaluable, and a - // literal the declared type can hold. + // A DEFAULT passes the same gate `CREATE` applies: evaluable, constant, and + // a literal the declared type can hold. 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(expr) = &column.default { validate_column_default( &DeclaredColumn { 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..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,7 +59,7 @@ pub(crate) fn parse_origin_column_def(s: &str) -> crate::Result Result<( /// Refuse one declared column `DEFAULT` the server cannot evaluate, or that /// the column's declared type cannot hold. +pub(super) fn validate_column_default( + column: &DeclaredColumn<'_>, + expr: &str, +) -> Result<(), DdlError> { + validate_column_default_clause("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 a carried guard clause the server cannot evaluate as the column +/// `DEFAULT` it becomes, or that the column's declared type cannot hold. +/// +/// `clause` names the keyword the author wrote, so a typeguard `VALUE` reports +/// itself rather than borrowing the `DEFAULT` wording. /// /// The expression is classified and parsed, never evaluated, so a /// `DEFAULT nextval('s')` column never advances its sequence at DDL time. A @@ -75,25 +114,28 @@ pub(super) fn validate_column_defaults(columns: &[(String, String)]) -> Result<( /// an INSERT coerces the materialized value; a generator or an expression has /// no value to check until it is evaluated. /// -/// An unregistered function name raises SQLSTATE `42883`, a literal the -/// declared type cannot represent `42804`, a literal past the declared -/// numeric width `22003`, and every other rejection `42601`. -pub(super) fn validate_column_default( +/// An unregistered function name raises SQLSTATE `42883`, a column-referencing +/// expression `42601`, a literal the declared type cannot represent `42804`, a +/// literal past the declared numeric width `22003`, and every other rejection +/// `42601`. +pub(super) fn validate_column_default_clause( + clause: &str, column: &DeclaredColumn<'_>, expr: &str, ) -> Result<(), DdlError> { + validate_constant_clause_expr(clause, column.name, expr)?; let compiled = CompiledDefault::declare(column.name, expr) - .map_err(|error| clause_error("DEFAULT", column.name, &error))?; + .map_err(|error| clause_error(clause, column.name, &error))?; let Some(literal) = compiled.literal() else { return Ok(()); }; let mut info = declared_column_info(column.name, column.declared_type); info.is_primary_key = column.primary_key; let value = default_value_to_sql(column.name, literal.clone()) - .map_err(|error| clause_error("DEFAULT", column.name, &error))?; + .map_err(|error| clause_error(clause, column.name, &error))?; coerce_write_literal(&info, value) .map(|_| ()) - .map_err(|error| clause_error("DEFAULT", column.name, &error)) + .map_err(|error| clause_error(clause, column.name, &error)) } /// 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 66fb8f94d..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; @@ -47,6 +48,27 @@ 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 { + // 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 } else if !coll.type_guards.is_empty() { typeguards_to_column_defs(&coll.type_guards)? @@ -61,6 +83,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 85fc6e48a..ffbb83280 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::{DeclaredColumn, validate_column_default}; +use super::super::column_default::{DeclaredColumn, validate_column_default_clause}; use super::support::err; use super::type_map::typeguard_type_to_column_type; @@ -16,8 +16,9 @@ use super::type_map::typeguard_type_to_column_type; /// REQUIRED fields become NOT NULL. DEFAULT expressions carry over. /// /// A carried-over expression becomes a column `DEFAULT`, so it passes the gate -/// every declared column `DEFAULT` passes. An unregistered function name -/// raises `42883`; every other rejection raises `42601`. +/// every declared column `DEFAULT` passes: an unregistered function name +/// raises `42883`, a literal the resolved type cannot hold `42804` or `22003`, +/// and every other rejection `42601`. pub(super) fn typeguards_to_column_defs( guards: &[nodedb_types::TypeGuardFieldDef], ) -> Result, DdlError> { @@ -49,11 +50,22 @@ 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()) { - // The resolved type's own spelling stands in for the declaration: - // a guard names no numeric width, so the canonical name resolves - // to the same width-less type the column will carry. - validate_column_default( + 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 { + // The one gate refuses an unregistered function name, a + // column-referencing expression and a literal the declared type + // cannot hold 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. The resolved type's own + // spelling stands in for the declaration: a guard names no numeric + // width, so the canonical name resolves to the same width-less + // type the column will carry. + validate_column_default_clause( + clause, &DeclaredColumn { name: &col.name, declared_type: &col.column_type.to_string(), 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 52e3fe66b..a0bfba5e2 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")))?; @@ -310,13 +316,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 fe4c86bc2..f701d8086 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")) @@ -67,7 +68,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))?; 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_default_expressions.rs b/nodedb/tests/wire/cases/sql_default_expressions.rs index cee4bbd03..5a41c4298 100644 --- a/nodedb/tests/wire/cases/sql_default_expressions.rs +++ b/nodedb/tests/wire/cases/sql_default_expressions.rs @@ -441,3 +441,75 @@ 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; + + // 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; +} + +/// The gate reads the parsed DEFAULT, never the definition text: a column name +/// that contains the word `default` must not be read as the clause, and a +/// constant DEFAULT beside such a name must pass. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_column_name_containing_default_is_not_read_as_a_clause() { + let server = TestServer::start().await; + server + .exec("CREATE COLLECTION def_named (id TEXT PRIMARY KEY, status TEXT)") + .await + .unwrap(); + + server + .exec("ALTER TABLE def_named ADD COLUMN is_default BOOLEAN") + .await + .expect("a column named is_default declares no DEFAULT clause"); + + server + .exec("ALTER TABLE def_named ADD COLUMN my_default TEXT DEFAULT 'x'") + .await + .expect("a constant DEFAULT beside a name that contains 'default' is accepted"); + + let rows = server + .query_text("SELECT is_default FROM def_named") + .await + .unwrap(); + assert!(rows.is_empty(), "no rows yet, got {rows:?}"); +} 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:?}" + ); +}