grammar+db: 3i — DML column-existence + cross-cut verification (ADR-0033 §8)

New dml_target_column_diagnostics pass: an ERROR for an unknown column
in the INSERT column list or the UPSERT DO UPDATE SET (validated
directly against the insert_target_table). The INSERT target isn't a
flat-scope `bindings` entry, so the existing schema-existence pass
didn't cover these; a targeted pass avoids the false INSERT…SELECT
ambiguity a global binding would cause.

Closes the 3i cross-cut "schema-existence fires on INSERT VALUES"
gate item, and closes the DA finding #12 (UPSERT DO UPDATE SET column
now flagged like a top-level UPDATE's SET column). Residual: bare
sql_expr_ident refs in the DO UPDATE SET RHS / WHERE remain
unvalidated for upserts (the documented flat-scope limitation).

Tests (+5): unknown INSERT column flagged + known silent; unknown
DO UPDATE SET column flagged + known/excluded silent; predicate
warning (= NULL) fires on a SQL UPDATE WHERE (cross-cut). 1596 pass /
0 fail / 1 ignored. Clippy clean.
This commit is contained in:
claude@clouddev1
2026-05-22 22:02:33 +00:00
parent 2d1112d0f3
commit cfd925c24a
+137
View File
@@ -1304,6 +1304,75 @@ fn dml_insert_arity_diagnostics(path: &MatchedPath) -> Vec<outcome::Diagnostic>
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<outcome::Diagnostic> {
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.