Repository navigation
feat(postgres): type json a lateral subquery or one case branch builds - #55
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Important
One narrow soundness hole in left_of: it can scope a LATERAL probe to the wrong relation and type a nullable field as non-null. It's a one-line fix, verified below. Apart from that, the LATERAL and CASE reading looks sound.
Reviewed changes
I reviewed the PR's single commit: the LATERAL and CASE paths in jsonshape.rs, the new oracle case, the re-blessed json_item_shapes/lateral.sql and the docs line.
- LATERAL subqueries:
lateral_targetresolvesx.col(or a barecolof a one-item FROM) to a LATERAL subquery's constructor target.lateralwalks that target in the subquery's own FROM and WHERE, and the wider probe reads onlyleft_ofthe subquery. The value is nullable under an outer join or a statement grouping set. - CASE:
case_branchreads a CASE whose one branch builds JSON and whose other branches areNULL.is_constructornow accepts such a CASE. - Oracle and docs: a new
json_lateral_casecase with 17 statements,lateral.sqlgoing fromjson?to{ j: { id: int8 } }, and the Postgres page updated for both.
These paths check out:
- The narrow probe runs first and the wider one only if it fails (
postgres.rsaround line 708). - A
left_ofthat returnsNonefalls back to the whole parent FROM, which can only widen nullability. - MERGE's missing source is a LEFT JOIN wrapper that
from_itemsreads as nulled. - Where
figurecan't name a column the way Postgres does, it gives?column?, never another real name.
claude-opus-5-5 ｜ 𝕏
da21187 to
10dff95
Compare
9f57ff8 to
4816106
Compare
There was a problem hiding this comment.
✅ No new issues found. Both earlier findings are fixed, and I've resolved their threads.
Reviewed changes
I reviewed the two commits added since the last review. The range-diff also shows changes from #53's branch, which this PR was rebased onto. Those belong to #53 and I left them out of this review.
- Aliased joins in
left_of:left_innow stops at a join that has its own alias, the same wayfrom_itemsdoes. The probe now reads themthat the subquery reads. The new oracle statementlateral_beside_aliased_join.sqland a unit assertion on the probe'sFROMcover this, and the statement is now typed{ o: { n: text? } }. - Simplified
case_branch: it now returnsOption<&Node>, and the walk always makes the branch nullable. The doc comment explains why: a CASE has at least oneWHEN, so if it has one branch that builds JSON, it also has aNULLbranch.
claude-opus-5-5 ｜ 𝕏
d9cc88c to
8bb2025
Compare
4816106 to
517218c
Compare
8bb2025 to
8a52f50
Compare
517218c to
f49bf30
Compare
There was a problem hiding this comment.
✅ No new issues found. This push rebases the PR onto
mainnow that #53 has merged. The only content change is to the docs.
Reviewed changes
I compared the new commits with the previously reviewed head 517218c using a range-diff. ccccdea and f49bf30 are identical to the commits I reviewed before. f479605 changes only the docs. jsonshape.rs, the oracle cases and engine.wasm did not change.
- Moved the docs into
main's newpostgres.mdxlayout: that layout uses<Rules>sections with bold headings. Thelateralwording is now part of the "Through awithquery or a subquery" rule, so alateralsubquery is no longer listed asunknown. No other page or the skill still says it is. - Added a "Through a
case" rule with a captured example:pg_json_case.sqlis captured asbig_order: { total: number | "NaN" | … } | nullinsite/lib/sql-modules.json. That matches the page's schema, whereorders.totalis a non-nullamount(numeric), and how the page says anumericappears inside JSON.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. The only new commit changes the docs example.
Reviewed changes
I reviewed the one commit added since f49bf30, 6361c95.
- Changed the
caseexample to build an integer field:pg_json_case.sqlnow buildsjsonb_build_object('id', id)instead oftotal, and its capture insite/lib/sql-modules.jsonis nowbig_order: { id: number } | null. That fits the page's schema, whereorders.idis abigserial, and the page's own rule that abigintinside JSON is anumber. The recapture changed only this entry.
claude-opus-5-5 ｜ 𝕏
6361c95 to
23d63bc
Compare
23d63bc to
a2ad4ae
Compare
Replaces #16, which GitHub closed when its base branch merged. It builds on #53 (the scoping fix), so this PR's base is that branch, and the diff here is this PR's commit alone. Merge #53 first.
#3 now types JSON that a WITH query or a plain derived table builds, through its own item statements. What it still left
unknownwas JSON built in alateralsubquery and JSON built in one branch of acase. Both are typed now:select m.id, x.items from member m left join lateral (select jsonb_agg(jsonb_build_object('name', t.name)) as items from team t where t.id = m.id) x on trueitems: json?items: [{ name: text }]?select x.snapshot from member m join lateral (select jsonb_build_object('name', t.name) as snapshot from team t where t.id = m.id) x on truejson{ name: text }select case when m.id > 1 then jsonb_build_object('n', m.first_name) end as o from member mjson?{ n: text }?jsonshape.rs,lateral_targetandlateral): a column of alateralsubquery whose target is a JSON constructor is read in the subquery's own context, itsfromandwhere. The wider probe reads only thefromitems left of the subquery (left_of), since the join that reads the subquery, aleft joinabove all, would null the subquery's own tables there. ANOT NULLcolumn inside stays non-null. An outer join over the subquery, or a grouping set of the statement, makes the value nullable. A subquery with a WITH of its own, a set operation, a*or grouping sets is not read, and a column list (x (b, a)) renames by position.case_branch): acasewhose one branch builds JSON and whose others areNULLhas that branch's shape, nullable where a branch isNULL. Two branches that build JSON may build two shapes, so they're not read.Pullfrog's two findings on #16, WITH-query scoping and alias column lists, concerned the WITH and derived-table code that #3 superseded. The scoping fixes are in #53, which covers the cases those findings named. This PR reads only
lateralsubqueries and handles their column lists.Tests: unit tests for the LATERAL probe and the CASE reading; the oracle case
json_lateral_case(17 statements, sound over seeded rows);json_item_shapes/lateral.sqlre-blessed fromjson?to{ j: { id: int8 } }; the full oracle with--strict-coverage, 102 cases, nothing else changed.