diff --git a/src/app.rs b/src/app.rs index 4bd6005..7c91f1f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -3353,6 +3353,8 @@ mod tests { vec![TableColumn { name: "price".to_string(), user_type: Type::Real, + not_null: false, + has_default: false, }], ); app.input = diff --git a/src/completion.rs b/src/completion.rs index f9a25c1..87f41be 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -55,10 +55,36 @@ pub struct SchemaCache { /// One column's user-facing type info, scoped to a table /// (ADR-0024 §Phase D, §WalkContext). +/// +/// `not_null` / `has_default` (ADR-0033 §8.3, sub-phase 3i) let the +/// walker pre-flight a `not_null_missing` WARNING — an `INSERT` +/// whose column list omits a required column. They default to +/// `false`, so callers/tests that don't care construct via +/// [`TableColumn::new`]. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct TableColumn { pub name: String, pub user_type: crate::dsl::types::Type, + /// The column is declared `NOT NULL` (a PK column is also + /// effectively not-null; the cache builder records that). + pub not_null: bool, + /// The column has a `DEFAULT` — so omitting it on `INSERT` is + /// fine even when `not_null`. + pub has_default: bool, +} + +impl TableColumn { + /// A column with no NOT-NULL / default info — the common case + /// for callers and tests that don't exercise ADR-0033 §8.3. + #[must_use] + pub fn new(name: impl Into, user_type: crate::dsl::types::Type) -> Self { + Self { + name: name.into(), + user_type, + not_null: false, + has_default: false, + } + } } impl SchemaCache { @@ -1453,6 +1479,8 @@ mod tests { .map(|(n, t)| TableColumn { name: (*n).to_string(), user_type: *t, + not_null: false, + has_default: false, }) .collect(); for c in &cols { @@ -1479,7 +1507,7 @@ mod tests { cache .table_columns .insert("Orders".to_string(), vec![ - TableColumn { name: "OrderTotal".to_string(), user_type: Type::Real }, + TableColumn { name: "OrderTotal".to_string(), user_type: Type::Real, not_null: false, has_default: false }, ]); cache.tables.push("Orders".to_string()); let cs = cands_with("update Customers set ", 21, &cache); @@ -2102,15 +2130,15 @@ mod tests { s.table_columns.insert( "a".to_string(), vec![ - TableColumn { name: "id".to_string(), user_type: Type::Int }, - TableColumn { name: "name".to_string(), user_type: Type::Text }, + TableColumn { name: "id".to_string(), user_type: Type::Int, not_null: false, has_default: false }, + TableColumn { name: "name".to_string(), user_type: Type::Text, not_null: false, has_default: false }, ], ); s.table_columns.insert( "b".to_string(), vec![ - TableColumn { name: "id".to_string(), user_type: Type::Int }, - TableColumn { name: "total".to_string(), user_type: Type::Real }, + TableColumn { name: "id".to_string(), user_type: Type::Int, not_null: false, has_default: false }, + TableColumn { name: "total".to_string(), user_type: Type::Real, not_null: false, has_default: false }, ], ); s diff --git a/src/dsl/walker/driver.rs b/src/dsl/walker/driver.rs index a5e730c..3b30639 100644 --- a/src/dsl/walker/driver.rs +++ b/src/dsl/walker/driver.rs @@ -2035,9 +2035,9 @@ mod tests { s.table_columns.insert( "users".to_string(), vec![ - TableColumn { name: "id".to_string(), user_type: Type::Int }, - TableColumn { name: "name".to_string(), user_type: Type::Text }, - TableColumn { name: "age".to_string(), user_type: Type::Int }, + TableColumn { name: "id".to_string(), user_type: Type::Int, not_null: false, has_default: false }, + TableColumn { name: "name".to_string(), user_type: Type::Text, not_null: false, has_default: false }, + TableColumn { name: "age".to_string(), user_type: Type::Int, not_null: false, has_default: false }, ], ); s diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 1543d36..0ddfa3b 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -1304,6 +1304,75 @@ fn dml_insert_arity_diagnostics(path: &MatchedPath) -> Vec diagnostics } +/// `not_null_missing` WARNING (ADR-0033 §8.3, sub-phase 3i). +/// +/// A SQL `INSERT` with an explicit `(column_name_list)` that omits a +/// column declared `NOT NULL` with no `DEFAULT` will be rejected by +/// the engine; this pre-flights it as a WARNING (advisory — the +/// statement still parses; the engine enforces it). `serial` / +/// `shortid` columns are excluded: they are auto-filled (engine +/// rowid / worker post-fill), so omitting them is correct, not a +/// missing required value. Only the explicit-column-list form is +/// covered (the no-list form's omission is an arity matter). +/// +/// Anchored on the target-table ident — there is no token for the +/// omitted column to point at. One WARNING per missing column. +fn dml_not_null_missing_diagnostics( + path: &MatchedPath, + schema: Option<&crate::completion::SchemaCache>, +) -> Vec { + use crate::dsl::grammar::IdentSource; + use crate::dsl::types::Type; + use outcome::{Diagnostic, MatchedKind, Severity}; + + let Some(schema) = schema else { + return Vec::new(); + }; + let Some((target, target_span)) = path.items.iter().find_map(|it| match it.kind { + MatchedKind::Ident { + source: IdentSource::Tables, + role: "insert_target_table", + } => Some((it.text.as_str(), it.span)), + _ => None, + }) else { + return Vec::new(); + }; + let listed: Vec = path + .items + .iter() + .filter_map(|it| match it.kind { + MatchedKind::Ident { + source: IdentSource::Columns, + role: "insert_column", + } => Some(it.text.to_ascii_lowercase()), + _ => None, + }) + .collect(); + if listed.is_empty() { + return Vec::new(); + } + let Some(cols) = schema.columns_for_table(target) else { + return Vec::new(); + }; + + let mut diagnostics = Vec::new(); + for c in cols { + let auto_generated = matches!(c.user_type, Type::Serial | Type::ShortId); + let required = c.not_null && !c.has_default && !auto_generated; + if required && !listed.contains(&c.name.to_ascii_lowercase()) { + diagnostics.push(Diagnostic { + severity: Severity::Warning, + span: target_span, + message: crate::friendly::translate( + "diagnostic.not_null_missing", + &[("column", &c.name as &dyn std::fmt::Display)], + ), + }); + } + } + diagnostics +} + /// SQL-expression predicate-warning pass (ADR-0032 §11.6 — the /// Phase-1 carry-over gap closure). /// @@ -2372,6 +2441,9 @@ fn walk_one_command<'a>( // with a VALUES tuple (per row) or the INSERT…SELECT // projection. d.extend(dml_insert_arity_diagnostics(&path)); + // ADR-0033 §8.3 — WARNING when an INSERT's column list omits + // a NOT-NULL-no-default (non-auto-gen) column. + d.extend(dml_not_null_missing_diagnostics(&path, ctx.schema)); // ADR-0032 §10.3 / §11.2 — diagnostics emitted during // the walk by node handlers with direct context the // post-walk passes can't reconstruct (primarily the @@ -3658,6 +3730,8 @@ mod tests { .map(|(n, t)| TableColumn { name: (*n).to_string(), user_type: *t, + not_null: false, + has_default: false, }) .collect(); let mut cache = SchemaCache::default(); @@ -4097,10 +4171,14 @@ mod tests { TableColumn { name: "id".to_string(), user_type: Type::Int, + not_null: false, + has_default: false, }, TableColumn { name: "name".to_string(), user_type: Type::Text, + not_null: false, + has_default: false, }, ], ); @@ -4110,10 +4188,14 @@ mod tests { TableColumn { name: "id".to_string(), user_type: Type::Int, + not_null: false, + has_default: false, }, TableColumn { name: "total".to_string(), user_type: Type::Real, + not_null: false, + has_default: false, }, ], ); @@ -4195,6 +4277,84 @@ mod tests { ); } + /// Like `schema_with`, but each column carries explicit + /// `(name, type, not_null, has_default)` so the not_null_missing + /// pass can be exercised. + fn schema_required(table: &str, cols: &[(&str, Type, bool, bool)]) -> SchemaCache { + let cols: Vec = cols + .iter() + .map(|(n, t, nn, hd)| TableColumn { + name: (*n).to_string(), + user_type: *t, + not_null: *nn, + has_default: *hd, + }) + .collect(); + let mut cache = SchemaCache::default(); + cache.tables.push(table.to_string()); + for c in &cols { + cache.columns.push(c.name.clone()); + } + cache.table_columns.insert(table.to_string(), cols); + cache + } + + #[test] + fn not_null_missing_fires_when_required_column_omitted() { + // `b` is NOT NULL with no default and is omitted → WARNING. + let schema = schema_required( + "t", + &[("a", Type::Int, false, false), ("b", Type::Text, true, false)], + ); + let diags = diag_keys("sqlinsert into t (a) values (1)", &schema); + assert!( + diags.iter().any(|d| d.contains("is required")), + "omitting NOT NULL `b` should warn; got {diags:?}", + ); + } + + #[test] + fn not_null_missing_silent_when_included() { + let schema = schema_required( + "t", + &[("a", Type::Int, false, false), ("b", Type::Text, true, false)], + ); + let diags = diag_keys("sqlinsert into t (a, b) values (1, 'x')", &schema); + assert!( + !diags.iter().any(|d| d.contains("is required")), + "including `b` must not warn; got {diags:?}", + ); + } + + #[test] + fn not_null_missing_silent_when_column_has_default() { + // NOT NULL but DEFAULT present → omitting is fine. + let schema = schema_required( + "t", + &[("a", Type::Int, false, false), ("b", Type::Text, true, true)], + ); + let diags = diag_keys("sqlinsert into t (a) values (1)", &schema); + assert!( + !diags.iter().any(|d| d.contains("is required")), + "a defaulted column must not warn; got {diags:?}", + ); + } + + #[test] + fn not_null_missing_excludes_auto_generated() { + // A serial PK is NOT NULL no-default but auto-filled, so + // omitting it is correct — not a missing required value. + let schema = schema_required( + "t", + &[("id", Type::Serial, true, false), ("b", Type::Text, false, false)], + ); + let diags = diag_keys("sqlinsert into t (b) values ('x')", &schema); + assert!( + !diags.iter().any(|d| d.contains("is required")), + "auto-gen serial must not warn; got {diags:?}", + ); + } + #[test] fn insert_arity_mismatch_single_row_fires() { let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]); @@ -4671,11 +4831,11 @@ mod tests { cache.columns.push("price".to_string()); cache.table_columns.insert( "a".to_string(), - vec![TableColumn { name: "id".to_string(), user_type: Type::Int }], + vec![TableColumn { name: "id".to_string(), user_type: Type::Int, not_null: false, has_default: false }], ); cache.table_columns.insert( "b".to_string(), - vec![TableColumn { name: "price".to_string(), user_type: Type::Real }], + vec![TableColumn { name: "price".to_string(), user_type: Type::Real, not_null: false, has_default: false }], ); let diags = diag_keys( "select * from a join b on price like 5", @@ -5044,10 +5204,14 @@ mod projection_before_from_tests { TableColumn { name: "real_col".to_string(), user_type: Type::Text, + not_null: false, + has_default: false, }, TableColumn { name: "another_col".to_string(), user_type: Type::Int, + not_null: false, + has_default: false, }, ], ); diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 0863eab..feb8dbe 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -45,6 +45,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("diagnostic.duplicate_cte", &["name"]), ("diagnostic.eq_null", &[]), ("diagnostic.insert_arity_mismatch", &["expected", "actual"]), + ("diagnostic.not_null_missing", &["column"]), ("diagnostic.like_numeric", &["column", "type"]), ("diagnostic.projection_alias_misplaced", &["alias", "clause"]), ("diagnostic.type_mismatch", &["column", "type"]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index e7a0b06..13cdda0 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -504,6 +504,7 @@ diagnostic: # ADR-0033 §8 — Phase-3 DML diagnostic keys. auto_column_overridden: "column `{column}` is auto-generated (`{type}`); providing an explicit value bypasses the auto-counter and may collide with later auto-generated values" insert_arity_mismatch: "the column list names {expected} column(s) but {actual} value(s) are given" + not_null_missing: "column `{column}` is required (`NOT NULL`, no default); the statement will fail when run" # Engine-error translations: an engine-rejected SQL statement # reaches the friendly-error layer (ADR-0019) and these keys diff --git a/src/input_render.rs b/src/input_render.rs index 62a436c..8123959 100644 --- a/src/input_render.rs +++ b/src/input_render.rs @@ -922,7 +922,7 @@ mod tests { cache.columns.push("order_col".to_string()); cache.table_columns.insert( "Orders".to_string(), - vec![TableColumn { name: "order_col".to_string(), user_type: Type::Int }], + vec![TableColumn { name: "order_col".to_string(), user_type: Type::Int, not_null: false, has_default: false }], ); let comp = crate::completion::candidates_at_cursor_in_mode( @@ -957,6 +957,8 @@ mod tests { .map(|(n, t)| TableColumn { name: (*n).to_string(), user_type: *t, + not_null: false, + has_default: false, }) .collect(); for c in &columns { diff --git a/src/runtime.rs b/src/runtime.rs index b2003ba..4669992 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1002,6 +1002,13 @@ async fn build_schema_cache(database: &Database) -> crate::completion::SchemaCac c.user_type.map(|ty| TableColumn { name: c.name, user_type: ty, + // A PK column is effectively NOT NULL even when + // PRAGMA's notnull flag isn't set on it + // (ADR-0033 §8.3); the not_null_missing pass + // excludes auto-gen types (serial/shortid) + // separately. + not_null: c.notnull || c.primary_key, + has_default: c.default.is_some(), }) }) .collect(); diff --git a/tests/typing_surface/mod.rs b/tests/typing_surface/mod.rs index 2ad3b97..4ea5c50 100644 --- a/tests/typing_surface/mod.rs +++ b/tests/typing_surface/mod.rs @@ -137,6 +137,8 @@ fn build_schema(tables: &[(&str, &[(&str, Type)])]) -> SchemaCache { .map(|(n, t)| TableColumn { name: (*n).to_string(), user_type: *t, + not_null: false, + has_default: false, }) .collect(); cache.tables.push((*table).to_string());