Repository navigation
fix(postgres): keep not null on columns read beside a set operation - #4
Conversation
|
No configured credential can serve this run. Your Claude Pro/Max subscription was rejected. Sign in again by running The providers' responses
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
05c81fd to
9fc930b
Compare
|
No configured credential can serve this run. Your Claude Pro/Max subscription was rejected. Sign in again by running The providers' responses
|
There was a problem hiding this comment.
ℹ️ No critical issues. One minor soundness edge case is noted inline.
Reviewed changes
This is the first Pullfrog review on this PR, so it covers both commits.
- Kept table provenance beside a set operation: a bare or qualified reference to a non-CTE table in the top-level
FROMis nowItemRead::Table.own_provenancetrusts only those columns when the statement holds a set operation, in bothsettleandchecks. - Split
target_readsfromitem_reads: per-target reads are now available without the "some query item is read" gate.Itemsnow tracksItem::{Query, Table, Other}through a sharedfind. - Settled recursive
withqueries that hold a parameter: thewith_stackself-read match now also comparesjsonshape::without_placeholdersforms, so a probe'sNULL-for-$ntext is recognized as the same query. - Added oracle coverage: added the
set_operation_provenancecase with 14 statements.timeouts/ancestorsnow typesidas non-null.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. The star-alignment gap from the prior review is closed.
Reviewed changes
One commit landed since the prior Pullfrog review: efb3ae7.
- Required at most one
*before trusting positions inown_provenance: with a matching length and at most one*, that*has to be exactly one column wide. So each read lines up with its column, which mirrors the width rule inEngine::item_columns. - Added a unit test and an oracle case:
own_provenance_lines_up_with_the_columnscovers one*that is one column wide, a zero-width*that leaves the lengths unequal, and two stars.with_union_two_stars.sqlpins{ t: text?, t2: text?, name: text? }.
claude-opus-5-5 ｜ 𝕏
25842f4 to
dffbe0d
Compare
A union anywhere in a statement, in a WITH query, a derived table or a WHERE subquery included, made every column nullable and dropped its CHECK list. Only a column read through the set operation is unsure of its provenance now; a bare read of a table in the statement's own FROM keeps its NOT NULL, and recursive WITH queries are typed by their arms.
Two `*` targets may be two columns and none, so as many targets as columns no longer shows which column each target is.
dffbe0d to
2ada6ba
Compare
A set operation anywhere in a Postgres statement made the engine distrust the provenance of every column it returned, because the server reports a set operation's columns as the leftmost arm's. So a
unioninside awithquery, a derived table or awhere … in (subquery), and everywith recursivequery, left each column of the statement| nulland dropped itsCHECKlist.Now only a column that may come through the set operation is treated that way. A bare reference to a column of a table in the statement's own
FROMkeeps itsNOT NULLand itsCHECKlist, whatever itswithqueries and subqueries hold, and a column read from awithquery keeps the nullability that query's own arms give it, as it already did for a derived table.with a as (select p.id, p.title from posts p union all select c.id, c.body from comments c) select id, title from aid: int8?, title: text?id: int8, title: textwith recursive tree as (… union all …) select id, name, depth from treeid: int8?, name: text?id: int8, name: textselect u.name from users u where u.id in (select user_id from posts union select 1)name: text?name: textwith a as (select 1 as x union select 2) select e.kind from events e, a(kindcheckedin ('click', 'key'))kind: text?kind: 'click' | 'key'with a as (select p.title as t from posts p union all select u.email from users u) select t from a(emailnullable)t: text?t: text?Columns read through a
*, a join with an alias of its own, or a bare name among severalFROMitems stay as they were.A recursive
withquery that holds a parameter also settles its columns' nullability by its arms now. A probe writesNULLwhere the statement has a placeholder, so the query's self-read inside a probe did not match the query being settled, and a counter such ast.depth + 1inwith recursive tree as (select …, 0 as depth from categories where id = :root union all select …, t.depth + 1 from categories c join tree t on …)stayedint4?; it isint4now.Tests:
oracle/cases/set_operation_provenance: 14 statements, the nullable-arm cases and a recursive counter among them, sound over the seeded edge values.oracle/cases/timeouts/ancestors.sql: the recursive query's primary key is now non-null.infer::target_reads).--strict-coverage, the loader's typecheck and tests, and the package build.