diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 0ddfa3b..5484a55 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 } +/// Unknown-column ERROR for the INSERT column list and the UPSERT +/// `DO UPDATE SET` columns (ADR-0033 §8.4 cross-cut + DA finding, +/// sub-phase 3i). +/// +/// The flat-scope `schema_existence_diagnostics` validates bare +/// columns against `bindings` built from `table_name`-role idents. +/// An INSERT's target uses the `insert_target_table` role, so it +/// isn't a binding — leaving `insert_column` (the `(a, b)` list) and +/// `update_set_column` (the `DO UPDATE SET` LHS) unvalidated. Adding +/// the target as a binding would falsely make a bare `INSERT … SELECT` +/// projection ambiguous against the target. This targeted pass +/// instead validates those two roles directly against the target's +/// columns — matching the parity a top-level `UPDATE`'s SET column +/// already has (`sql_update_unknown_set_column_is_error`). +/// +/// Only applies to statements with an `insert_target_table` (INSERT +/// / UPSERT); a top-level `UPDATE`/`DELETE` has no such ident, so its +/// columns continue through the binding-based pass unchanged. +fn dml_target_column_diagnostics( + path: &MatchedPath, + schema: Option<&crate::completion::SchemaCache>, +) -> Vec { + use crate::dsl::grammar::IdentSource; + use outcome::{Diagnostic, MatchedKind, Severity}; + + let Some(schema) = schema else { + return Vec::new(); + }; + let Some(target) = path.items.iter().find_map(|it| match it.kind { + MatchedKind::Ident { + source: IdentSource::Tables, + role: "insert_target_table", + } => Some(it.text.as_str()), + _ => None, + }) else { + return Vec::new(); + }; + // Unknown target table is flagged on its own slot elsewhere; if + // we can't resolve its columns, defer rather than false-flag. + let Some(cols) = schema.columns_for_table(target) else { + return Vec::new(); + }; + + let mut diagnostics = Vec::new(); + for item in &path.items { + let MatchedKind::Ident { + source: IdentSource::Columns, + role: "insert_column" | "update_set_column", + } = item.kind + else { + continue; + }; + if !cols.iter().any(|c| c.name.eq_ignore_ascii_case(&item.text)) { + diagnostics.push(Diagnostic { + severity: Severity::Error, + span: item.span, + message: crate::friendly::translate( + "diagnostic.unknown_column", + &[ + ("name", &item.text as &dyn std::fmt::Display), + ("table", &target as &dyn std::fmt::Display), + ], + ), + }); + } + } + diagnostics +} + /// `not_null_missing` WARNING (ADR-0033 §8.3, sub-phase 3i). /// /// A SQL `INSERT` with an explicit `(column_name_list)` that omits a @@ -2444,6 +2513,11 @@ fn walk_one_command<'a>( // 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-0033 §8.4 cross-cut — ERROR for an unknown column in + // the INSERT column list or the UPSERT DO UPDATE SET (the + // INSERT target isn't a binding, so the flat-scope pass + // doesn't cover these). + d.extend(dml_target_column_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 @@ -4299,6 +4373,69 @@ mod tests { cache } + #[test] + fn insert_column_list_unknown_column_is_flagged() { + // 3i cross-cut: schema-existence fires on the INSERT column + // list (the target isn't a binding, so a targeted pass covers + // it). + let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); + let diags = diag_keys("sqlinsert into t (nonexistent) values (1)", &schema); + assert!( + diags.iter().any(|d| d.contains("no such column")), + "unknown INSERT column should be flagged; got {diags:?}", + ); + } + + #[test] + fn insert_column_list_known_columns_silent() { + let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); + let diags = diag_keys("sqlinsert into t (a, b) values (1, 'x')", &schema); + assert!( + !diags.iter().any(|d| d.contains("no such column")), + "known columns must not flag; got {diags:?}", + ); + } + + #[test] + fn upsert_do_update_unknown_set_column_is_flagged() { + // DA finding (#12) closed: parity with a top-level UPDATE's + // SET column — an unknown DO UPDATE SET column is flagged. + let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); + let diags = diag_keys( + "sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set nosuch = 1", + &schema, + ); + assert!( + diags.iter().any(|d| d.contains("no such column")), + "unknown DO UPDATE SET column should be flagged; got {diags:?}", + ); + } + + #[test] + fn upsert_do_update_known_set_column_silent() { + let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]); + let diags = diag_keys( + "sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = excluded.b", + &schema, + ); + assert!( + !diags.iter().any(|d| d.contains("no such column")), + "known SET column + excluded ref must be clean; got {diags:?}", + ); + } + + #[test] + fn predicate_warning_fires_on_sql_update_values_context() { + // 3i cross-cut: the Phase-2 predicate-warning pass fires on a + // DML SET/WHERE sql_expr slot (here `= NULL` in an UPDATE). + let schema = schema_with("t", &[("id", Type::Int), ("v", Type::Int)]); + let diags = diag_keys("sql_update t set v = 1 where v = NULL", &schema); + assert!( + diags.iter().any(|d| d.contains("IS NULL")), + "eq_null should fire on the UPDATE WHERE; got {diags:?}", + ); + } + #[test] fn not_null_missing_fires_when_required_column_omitted() { // `b` is NOT NULL with no default and is omitted → WARNING.