fix(sql): raise 23502 for NULL primary keys instead of committing them

[EnRaiha]

fix(sql): raise 23502 for NULL primary keys instead of committing them

Fixes #293

Problem

A NULL primary key — explicit, or by omitting the column — committed on the document (schemaless) and kv engines. Uniqueness still applied to real values, so the key was neither unique nor non-null for those rows, and several NULL-keyed rows coexisted per collection. Readback was silently inconsistent: SELECT and DELETE found all NULL-keyed rows while count(*) found one — one row stored an explicit null, the other had the field absent, and the two paths treated absence differently.

Fix

PRIMARY KEY implies NOT NULL at plan time on every engine with a declared key. INSERT, UPSERT, and INSERT ... ON CONFLICT rows are checked after type coercion, before engine dispatch:

Input	Before	After
INSERT ... (id, v) VALUES (NULL, ...) declared doc	committed NULL-key row	23502
INSERT ... (v) VALUES (...) (pk omitted), declared doc	committed NULL-key row	23502
Same on kv	committed empty-key entry	23502
Same on document_strict	internal serialization error (binary_tuple)	clean 23502
UPSERT with NULL key (doc/kv)	committed	23502
Auto-id / renamed-PK-only schemaless, key omitted	fresh identity minted	unchanged (synthetic-key exemption)
Duplicate real key	23505 uniqueness	unchanged

A row that omits the declared key or binds it to NULL used to fall through to the "fresh surrogate" identity path — silently minting a new identity for what the caller declared as the row's key. Collections whose key is synthetic legitimately mint fresh identities and are exempt: _rowid collections and schemaless collections created without a column list (their only catalog column is the auto PK, matching the #292 closed-schema line).

The error is typed end to end: SqlError::NotNullViolation → crate::Error::NotNullViolation → public NodeDbError code 1207 / msgpack tag 80 → SQLSTATE 23502 on pgwire and native, mirroring the undefined_function path.

Out of scope

• Pre-existing NULL-keyed rows committed before this fix remain; repair of existing data is tracked separately.
• INSERT ... SELECT sources flow through a separate conversion path; a NULL key there still takes the old surrogate route (tracked separately).
• Raw kv collections (no declared PK, sentinel key column): a NULL/omitted key is rejected by the same gate — the key bytes are the identity there too.
• PRIMARY KEY DEFAULT NULL (pathological DDL) can still reach the surrogate route when the default evaluates to nothing; noted, not chased.

How to test

cargo test -p nodedb-sql --lib
cargo test -p nodedb-types --lib
cargo nextest run -p nodedb --test wire -E 'test(not_null_pk_23502)'

Verified

• Live matrix on fresh main + this branch: 23502 on doc/kv/strict/columnar for explicit-NULL and omitted keys across INSERT/UPSERT; uniqueness (23505) unchanged; auto-id exemption intact.
• Wire not_null_pk_23502: 4 passed.
• Sentinels (kv defaults, schemaless surface, insert value expressions, KV/insert RETURNING): 47 passed.
• cargo fmt --all -- --check clean; clippy -D warnings clean on nodedb-sql, nodedb-types, nodedb; unit 864 + 688 passed.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.