fix(analyzer): stop promising NOT NULL from rows a join or view may not have - #63
Merged
Merged
Conversation
- Count the set-returning calls of ORDER BY, GROUP BY and DISTINCT ON items that aren't select-list expressions with the select list's: PG runs them in the level's ProjectSet, where they can leave the level without rows or pad the select list's calls with NULL. This covers the row guarantees (finish_level, FROM-less subqueries), the lockstep padding, EXCEPT's always-NULL arm and the FOR UPDATE check. - Reject a VALUES list sorted by a set-returning call where it is sure to be planned: every execution fails with "set-valued function called in context that cannot accept a set". - Follow a foreign key only through the equality it enforces: the query's `=` must resolve to the key's pfeqop (derived from the referenced unique index's operator family) or its commutator, and that operator must be immutable (`timestamptz = timestamp` depends on the TimeZone). - Re-analyze a view's stored query only while its names still resolve to the relations, columns, functions, operators and types they did at CREATE VIEW (stored per view), for view origins, view nullability refreshes and rows written through views. - Treat a statement reading a view with a locking clause (directly or through nested views) as locking rows, and re-derive a view's column nullability under row locking when the statement locks rows. Co-Authored-By: Claude <noreply@anthropic.com>
lbguilherme
force-pushed
the
fix/join-soundness
branch
from
October 3, 2026 11:34
16fdee7 to
e90f353
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The join and row-guarantee narrowing promised NOT NULL for values PostgreSQL 18 returns as NULL. A soundness review reproduced six ways it happens:
SELECT (SELECT 1 ORDER BY generate_series(1,0))is NULL, andSELECT generate_series(1,2) AS g ORDER BY generate_series(1,3)padsgwith NULL.=. Forchar(3)referenced bytext, the key compares withbpchareq(trailing blanks ignored), but the query'spb.k = cb.kresolves totext = textand can miss.timestamptz = timestampis STABLE, so rows checked under UTC can stop matching in another zone.search_path, or swapping column names, re-analysis read a different table or column. That fed FK following, view origins, view nullability refreshes and rows written through views.CREATE VIEW cv AS SELECT * FROM c FOR UPDATE, EvalPlanQual re-fetches a concurrently updated row whose new parent the snapshot can't see.VALUES (1) ORDER BY generate_series(1,0)was accepted, but every execution fails.(Item 5,
DISABLE TRIGGER ALLon partitions, is handled in a separate PR.)Fix
level_srf_callscollects the set-returning calls of the select list plus those in sort, grouping and DISTINCT ON items that don't repeat a select-list expression. It is used wherever only the select list was counted before: row guarantees, FROM-less subqueries, lockstep padding, EXCEPT's always-NULL arm, and the FOR UPDATE check.=resolves to the key'spfeqop(or to its commutator when written the other way round), and only when that operator is immutable.pfeqopis derived asATAddForeignKeyConstraintdoes, from the referenced unique index's operator family; only no-op casts are allowed for the referencing value. If several unique indexes cover the key, they must all agree, becauseconindidisn't recorded.reanalyze_view) is trusted only while the query still resolves to the same objects; otherwise it returnsNone, which is the conservative path every caller already had. View ASTs are still not rewritten on RENAME. A benign rename therefore still loses narrowing through that view, as it did before.SELECT vs.name FROM vs FOR UPDATEreturned NULL on PG under a concurrent re-key, so such columns are now re-derived under row locking.WHERE false, an unreferenced CTE,EXISTSorCASE WHEN false; those cases stay accepted and are tested.Tests
typedpg_analyzer/tests/query/join_soundness.rshas 17 tests. Before the fix, all 13 regression tests fail. The 4 near-miss tests pass both before and after: cross-type, domain, varchar and date keys are still followed; a sort key with no new SRF still narrows; and a view whose names still resolve the same way still narrows.Verification:
cargo nextest run --release -p typedpg_analyzer: all passscripts/run-pg-sanity.sh --no-fail-fast(full oracle on PG 18): 2266 passed, no "nullability unsound"cargo nextest run --release --workspace: 2522 passedcargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings, andcargo clippy -p typedpg_analyzer --all-targets --features pg_sanity -- -D warnings: cleanImplemented by Claude Opus 5.5 (claude-opus-5-5) in Claude Code via T3 Code.