grammar+db: 3i — not_null_missing diagnostic + TableColumn constraints (ADR-0033 §8.3)
Extend SchemaCache TableColumn with not_null + has_default (with a TableColumn::new constructor for the common no-constraint case), populated in build_schema_cache from ColumnDescription (a PK column counts as not-null). New dml_not_null_missing_diagnostics pass: a WARNING when a SQL INSERT's explicit column list omits a column that is NOT NULL with no DEFAULT — advisory (the engine enforces it). serial/shortid (auto-filled) and defaulted columns are excluded. Anchored on the target-table ident (no token for the omitted column). Catalog key diagnostic.not_null_missing (engine-neutral). Tests (+4): fires on omitted required column; silent when included, when defaulted, and for auto-gen serial/shortid. ~24 TableColumn literal sites updated for the two new fields (build clean). 1591 pass / 0 fail / 1 ignored. Clippy clean. All three ADR-0033 §8 DML diagnostics now implemented. Remaining 3i: cross-cut verification + #12 UPSERT DO UPDATE validation.
This commit is contained in:
@@ -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
|
||||
|
||||
+166
-2
@@ -1304,6 +1304,75 @@ fn dml_insert_arity_diagnostics(path: &MatchedPath) -> Vec<outcome::Diagnostic>
|
||||
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<outcome::Diagnostic> {
|
||||
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<String> = 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<TableColumn> = 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,
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user