grammar+db: 3i — auto_column_overridden diagnostic (ADR-0033 §8.2)
New dml_auto_column_diagnostics pass: a WARNING when a SQL INSERT's explicit column list names a serial/shortid (auto-generated) column — the explicit value bypasses the auto-counter/generator and may collide with later auto-generated values. Advisory only (ADR-0027 §1); the statement still runs. Conflict-target columns (distinct conflict_target_column role) are not mistaken for inserted columns. Catalog key diagnostic.auto_column_overridden (engine-neutral). Tests (+4): serial + shortid fire; omitted is silent; ON CONFLICT target not falsely flagged. 1580 pass / 0 fail / 1 ignored. Clippy clean. Remaining 3i: insert_arity_mismatch, not_null_missing (needs TableColumn not_null+default), cross-cut verification, #12 UPSERT DO UPDATE validation.
This commit is contained in:
@@ -1106,6 +1106,72 @@ fn compound_arity_diagnostics(
|
||||
diagnostics
|
||||
}
|
||||
|
||||
/// `auto_column_overridden` WARNING (ADR-0033 §8.2, sub-phase 3i).
|
||||
///
|
||||
/// A SQL `INSERT` whose explicit column list names a `serial` or
|
||||
/// `shortid` column is providing a value for a column the playground
|
||||
/// auto-generates (`serial` via the engine rowid, `shortid` via the
|
||||
/// worker post-fill). That bypasses the auto-counter / generator and
|
||||
/// can collide with later auto-generated values — pedagogically worth
|
||||
/// a nudge, but advisory: the statement still runs (ADR-0027 §1).
|
||||
///
|
||||
/// The conflict-target column list of an `ON CONFLICT (…)` uses a
|
||||
/// distinct role (`conflict_target_column`), so it is not mistaken
|
||||
/// for an inserted column here.
|
||||
fn dml_auto_column_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) = 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();
|
||||
};
|
||||
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",
|
||||
} = item.kind
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
if let Some(col) = cols
|
||||
.iter()
|
||||
.find(|c| c.name.eq_ignore_ascii_case(&item.text))
|
||||
&& matches!(col.user_type, Type::Serial | Type::ShortId)
|
||||
{
|
||||
diagnostics.push(Diagnostic {
|
||||
severity: Severity::Warning,
|
||||
span: item.span,
|
||||
message: crate::friendly::translate(
|
||||
"diagnostic.auto_column_overridden",
|
||||
&[
|
||||
("column", &item.text as &dyn std::fmt::Display),
|
||||
("type", &col.user_type as &dyn std::fmt::Display),
|
||||
],
|
||||
),
|
||||
});
|
||||
}
|
||||
}
|
||||
diagnostics
|
||||
}
|
||||
|
||||
/// SQL-expression predicate-warning pass (ADR-0032 §11.6 — the
|
||||
/// Phase-1 carry-over gap closure).
|
||||
///
|
||||
@@ -2167,6 +2233,9 @@ fn walk_one_command<'a>(
|
||||
// operator slot is highlighted rather than the engine
|
||||
// wording shown at execution time.
|
||||
d.extend(compound_arity_diagnostics(&path));
|
||||
// ADR-0033 §8.2 — WARNING when a SQL INSERT lists a
|
||||
// serial/shortid (auto-generated) column explicitly.
|
||||
d.extend(dml_auto_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
|
||||
@@ -3990,6 +4059,54 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_column_overridden_fires_on_explicit_serial() {
|
||||
// ADR-0033 §8.2: listing a serial column explicitly warns.
|
||||
let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]);
|
||||
let diags = diag_keys("sqlinsert into t (id, name) values (5, 'x')", &schema);
|
||||
assert!(
|
||||
diags.iter().any(|d| d.contains("auto-generated")),
|
||||
"explicit serial value should warn; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_column_overridden_fires_on_explicit_shortid() {
|
||||
let schema = schema_with("t", &[("id", Type::ShortId), ("name", Type::Text)]);
|
||||
let diags = diag_keys("sqlinsert into t (id, name) values ('abc', 'x')", &schema);
|
||||
assert!(
|
||||
diags.iter().any(|d| d.contains("auto-generated")),
|
||||
"explicit shortid value should warn; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_column_overridden_silent_when_omitted() {
|
||||
// Negative: omitting the auto-gen column does not warn.
|
||||
let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]);
|
||||
let diags = diag_keys("sqlinsert into t (name) values ('x')", &schema);
|
||||
assert!(
|
||||
!diags.iter().any(|d| d.contains("auto-generated")),
|
||||
"omitting the serial column must not warn; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_column_overridden_conflict_target_not_flagged() {
|
||||
// The ON CONFLICT (id) target names `id` but isn't an
|
||||
// *inserted* value, so it must not trigger the warning;
|
||||
// only the inserted `id` in the column list would.
|
||||
let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]);
|
||||
let diags = diag_keys(
|
||||
"sqlinsert into t (name) values ('x') on conflict (id) do nothing",
|
||||
&schema,
|
||||
);
|
||||
assert!(
|
||||
!diags.iter().any(|d| d.contains("auto-generated")),
|
||||
"conflict-target serial column must not warn; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upsert_excluded_resolves_in_do_update() {
|
||||
// ADR-0033 §9: inside a DO UPDATE action, `excluded.col`
|
||||
|
||||
@@ -39,6 +39,7 @@
|
||||
pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
||||
// ---- Pre-submit diagnostics (ADR-0027) ----
|
||||
("diagnostic.ambiguous_column", &["column", "qualifiers"]),
|
||||
("diagnostic.auto_column_overridden", &["column", "type"]),
|
||||
("diagnostic.compound_arity_mismatch", &["op", "left_n", "right_n"]),
|
||||
("diagnostic.cte_arity_mismatch", &["cte", "declared", "actual"]),
|
||||
("diagnostic.duplicate_cte", &["name"]),
|
||||
|
||||
@@ -501,6 +501,8 @@ diagnostic:
|
||||
cte_arity_mismatch: "CTE `{cte}` declares {declared} columns but its body has {actual}"
|
||||
compound_arity_mismatch: "`{op}` requires both sides to have the same number of columns — left has {left_n}, right has {right_n}"
|
||||
duplicate_cte: "duplicate `WITH` table name: `{name}`"
|
||||
# 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"
|
||||
|
||||
# Engine-error translations: an engine-rejected SQL statement
|
||||
# reaches the friendly-error layer (ADR-0019) and these keys
|
||||
|
||||
Reference in New Issue
Block a user