Skip to content

fix(analyzer): stop promising NOT NULL for DML rows PostgreSQL stores as NULL - #61

Merged
lbguilherme merged 1 commit into
mainfrom
fix/dml-soundness
Oct 3, 2026
Merged

lbguilherme merged 1 commit into
mainfrom
fix/dml-soundness

Conversation

@lbguilherme

Copy link
Copy Markdown
Member

A soundness review of the DML area found ten ways INSERT / UPDATE / DELETE / MERGE could be typed NOT NULL where PostgreSQL 18 returns NULL (or rejected a statement PG runs). Each was reproduced against PG 18.6:

  1. OVERRIDING USER VALUE — the identity value given was taken as stored (INSERT INTO idt(id, a) OVERRIDING USER VALUE VALUES (-5, NULLIF(1,1)) RETURNING a with CHECK (id > 0 OR a IS NOT NULL) said a NOT NULL), and VALUES (NULL, …) there was rejected. USER and SYSTEM VALUE had been collapsed into one flag.
  2. SRF in a one-row VALUES — WITH i AS (INSERT … VALUES (generate_series(1, 0)) RETURNING id) was taken to return exactly one row.
  3. View on a view — a row written through v2 (over v1 … WHERE a IS NOT NULL) was narrowed by v1's WHERE.
  4. DO INSTEAD rules with RETURNING on a table (or a table under a view) were ignored; RETURNING then reads the rule's rows.
  5. Assignment casts that can return NULL (time(timestamp) on infinity, a user's cast) kept the value non-NULL in INSERT, UPDATE, MERGE, DEFAULT, INSERT … SELECT and generated columns (so plain SELECT gen_col too).
  6. A view exposing a base column twice picked one of them at random (HashMap order).
  7. Typmod coercions of constants — numeric(2,-1) stores 15 as 20 and varchar(2) drops 'ab ''s trailing space, yet the literal was recorded as stored and refuted CHECK constraints.
  8. FK guarantees on the written row — RETURNING through a view trusted the new row's foreign key, though its parent may be one the statement's snapshot doesn't see (inserted by a CTE of the same statement).
  9. Intermediate view defaults were ignored when the outer view doesn't expose the column.
  10. (false rejection) A literal NULL into a NOT NULL column was rejected even when a BEFORE ROW trigger fills it before ExecConstraints runs.

While fixing 6 and 9 I found two more of the same family, now fixed and tested: a domain-typed view column takes its type's default at the view level (not the base column's DEFAULT NULL), and UPDATE view SET col = DEFAULT is the view's own default or NULL — rewriteTargetListIU doesn't fall back to the base default.

Fix

  • Overriding (NotSet / UserValue / SystemValue) replaces the boolean through INSERT, MERGE and the rewriter; under USER VALUE an identity column stores its sequence value and a NULL given for it is accepted.
  • insert_returns_one_row requires no set-returning function in the row.
  • written_row_attrs re-reads a view's body with its FROM entry marked as the written row (with_written_relation): an inner view's columns recurse as written rows (no WHERE narrowing), and the base scan carries no Origin, so no FK reasoning applies to it.
  • returning_rewritten walks the target and its view chain; an INSTEAD rule for the event (or an INSTEAD OF trigger) makes every RETURNING column nullable — INSERT, UPDATE, MERGE and DELETE.
  • ValueInfo::assigned replaces ValueInfo::of: nullability goes through expr::assignment_nullable (which calls cast_function_can_return_null, left unchanged), and a constant is recorded only when no user cast runs and typmod::keeps_literal proves the typmod coercion is the identity. The same check applies to INSERT … SELECT and generation expressions.
  • WriteTarget keeps every level of the view chain: omitted takes the first level whose columns storing the base column have a default (or a domain type), all such columns count when a view exposes one twice, and default_of (UPDATE) is the target's own default.
  • null_assignment_error skips the column NOT NULL (not a NOT NULL domain, which fails before triggers) when a BEFORE ROW trigger for the event can rewrite the row — on the table, a partition the row is routed to, or (UPDATE) a BEFORE INSERT one on a partition a row may move to. CLAUDE.md's literal-NULL paragraph and the README's RETURNING section are updated accordingly.

Tests

typedpg_analyzer/tests/query/dml_soundness.rs — 15 tests, each with the repros and the near misses that must keep narrowing (plain identity inserts, plain one-row VALUES, stored view rows and DELETE through views, ALSO rules and rules for other events, casts that can't return NULL, constants a typmod keeps, AFTER triggers / NOT NULL domains / partitions without a trigger still rejected). 14 of them fail on main; the 15th pins near misses only.

Verification

  • cargo nextest run --release -p typedpg_analyzer — 2260 passed
  • scripts/run-pg-sanity.sh --no-fail-fast (PG 18) — 2264 passed, no nullability unsound; 2114 queries executed over adversarial data
  • cargo nextest run --release --workspace — 2520 passed
  • cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, cargo clippy -p typedpg_analyzer --all-targets --features pg_sanity -- -D warnings — clean

Implemented by Claude Opus 5.5 (claude-opus-5-5) in Claude Code via T3 Code.

A soundness review found INSERT / UPDATE / DELETE / MERGE shapes where the
analyzer inferred NOT NULL for a value PG 18 returns NULL for, or rejected
a statement PG runs:

- OVERRIDING USER VALUE: the identity value given was taken as stored, and
  a NULL there was rejected. USER and SYSTEM VALUE are now kept apart;
  under USER VALUE an identity column stores its sequence's value.
- A one-row VALUES with a set-returning function made a data-modifying
  CTE return exactly one row.
- A row written through a view of a view was narrowed by the inner view's
  WHERE, and its foreign keys vouched for a parent the statement's
  snapshot may not see: the view body is re-read with its FROM entry as
  the written row (inner views not narrowed, the base scan without
  origin).
- A DO INSTEAD rule (or INSTEAD OF trigger) on the target or down its view
  chain was ignored when typing RETURNING, which then reads the rule's
  rows: nothing is NOT NULL there.
- An assignment coercion through a cast that can return NULL
  (time(timestamp) on infinity, a user's cast) kept the value non-NULL in
  INSERT, UPDATE, MERGE, DEFAULT and generated columns.
- A view exposing a base column twice picked one at random; every view
  down the chain supplies its defaults (a domain-typed view column its
  type's), and UPDATE ... SET col = DEFAULT through a view is the view's
  default or NULL.
- A constant was recorded as stored although a typmod coercion changes it
  (numeric(2,-1) rounds 15, varchar(2) drops a trailing space).
- A literal NULL into a NOT NULL column was rejected although a BEFORE ROW
  trigger for the event can fill it before ExecConstraints runs.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant