db+walker: 3i DA pass — not_null PK false-positive fix + arity hardening

DA pass on 3i. Fix: build_schema_cache set not_null = c.notnull ||
c.primary_key, which would false-flag an omitted `int` PK as a
not_null_missing WARNING — but an int PK is an INTEGER PRIMARY KEY
rowid alias that auto-fills (and SQLite's PK-NULL quirk means a PK
isn't implicitly NOT NULL anyway). Use c.notnull alone (ADR-0033
§8.3 "declared NOT NULL"): faithful and false-positive-free.

Arity-walk hardening (same class as the ON CONFLICT regression the
existing tests caught mid-3i): RETURNING after VALUES is a depth-0
keyword that ends the tuple list (only the real tuple is flagged),
and a comma nested in a function-call value (depth ≥ 2) does not
inflate the tuple's value count.

Tests (+2). 1598 pass / 0 fail / 1 ignored. Clippy clean.
This commit is contained in:
claude@clouddev1
2026-05-22 22:06:04 +00:00
parent cfd925c24a
commit 4fa0aa06e9
2 changed files with 32 additions and 6 deletions
+23
View File
@@ -4554,6 +4554,29 @@ mod tests {
);
}
#[test]
fn insert_arity_with_returning_flags_only_tuple() {
// RETURNING is a depth-0 keyword that ends the VALUES clause;
// the walk must stop there (same guard as ON CONFLICT) so the
// RETURNING projection isn't mis-counted as a value tuple.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys("sqlinsert into t (a, b) values (1, 2, 3) returning *", &schema);
let n = diags.iter().filter(|d| d.contains("value(s) are given")).count();
assert_eq!(n, 1, "only the 3-value tuple is flagged; got {diags:?}");
}
#[test]
fn insert_arity_ignores_commas_nested_in_a_value() {
// A comma inside a function call (depth ≥ 2) is not a tuple
// separator and must not inflate the value count.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys("sqlinsert into t (a, b) values (1, coalesce(1, 2))", &schema);
assert!(
!diags.iter().any(|d| d.contains("value(s) are given")),
"two values (one a 2-arg call) match the 2-col list; got {diags:?}",
);
}
#[test]
fn insert_select_arity_mismatch_fires() {
// INSERT … SELECT: 1 listed column vs a 2-item projection.
+9 -6
View File
@@ -1002,12 +1002,15 @@ 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,
// Only a column the engine reports as NOT NULL
// (ADR-0033 §8.3 "declared NOT NULL"). We do
// NOT treat a PK as implicitly not-null: an
// `int` PK is an `INTEGER PRIMARY KEY` rowid
// alias that auto-fills, so flagging it omitted
// would be a false positive (SQLite reports
// notnull=0 for it anyway). serial/shortid are
// excluded by type in the not_null_missing pass.
not_null: c.notnull,
has_default: c.default.is_some(),
})
})