Repository navigation
feat(postgres): carry json shapes through subqueries, with and case - #16
colinhacks wants to merge 3 commits into
Conversation
- jsonb_agg(t.*), json_agg(t.*) filter (...), jsonb_build_object('r', t.*)
and to_jsonb(t.*) are the whole row, not the row's first column
- an array the statement builds keeps its null elements inside json
- a float or numeric inside json may be "NaN", "Infinity" or "-Infinity",
and a row whose columns were not read is an object there
- two result columns of one name are an error (reylo14) at the second,
and the row type keeps the last, as drivers do
- the oracle checks each value's kind inside a json document
Found by running SafeQL's test statements through the oracle.
- a column read from a subquery in from, a with query or a lateral subquery takes the shape of the json that item builds, nullable where an outer join can null the item - a case whose one branch builds json and whose others are null takes that branch's shape, nullable
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Important
derived_target decides which query a name reads without Postgres's scoping rules. A WITH query whose body reads a table of its own name crashes the engine, and three other cases produce shapes the database does not return. I reproduced all four with temporary shapes/probes_ahead unit tests at this commit.
Reviewed changes
This review covers commit 998697d, which carries JSON shapes through FROM items, WITH queries, LATERAL subqueries and CASE.
- Columns of a FROM item:
derived_targetresolvesx.col(or a barecolwhen FROM holds one item) to a target in a subquery or top-level WITH query that builds JSON.derivedthen walks that target in the item's own FROM/WHERE context and marks it nullable under an outer join. CASE: when one branch builds JSON and every other branch isNULL(a missingELSEcounts asELSE NULL), the column takes that branch's shape, nullable.- Naming: an unaliased column ref is named after its last field, and an unaliased
CASEis namedcase. - Oracle and docs: adds
oracle/cases/json_derivedand a sentence on the Postgres docs page.
ℹ️ The diff also includes the base PR's commits
json-soundness has been rebased (now 0e78008, whose fix commit is a09a744), but this branch still sits on the old copies (566c930, 95de6c2). GitHub therefore shows the base PR's whole-row, reylo14 and non-finite-number changes as part of this PR. Rebasing onto the current json-soundness will reduce the diff to 998697d.
claude-opus-5-5 ｜ 𝕏
| }; | ||
| let body = match item.node.as_ref()? { | ||
| NodeEnum::RangeSubselect(r) => r.subquery.as_deref().and_then(as_select)?.clone(), | ||
| NodeEnum::RangeVar(rv) if rv.schemaname.is_empty() => self.ctes.iter().find(|(name, _)| *name == rv.relname)?.1.clone(), |
There was a problem hiding this comment.
A RangeVar is resolved against self.ctes, which holds every top-level WITH query by name, wherever the reference stands. Postgres scopes these names, and this lookup gets the following wrong:
- Crash:
with t as (select jsonb_build_object('n', t.o) as o from t) select o from t. Inside the body,tis a table, since a non-recursive WITH query cannot read itself or a later sibling. Here it resolves back to the CTE, soderivedandderived_targetrecurse forever and the engine aborts with a stack overflow. This runs fromprobes_aheadbefore any describe, so the statement does not even need to be valid. - Wrong shape (shadowing): a nested
with x as (…)inside a subquery shadows a top-levelx, but the outerx's shape is used. - Wrong shape (DML target):
with users as (select jsonb_build_object('k', 1) as doc) insert into users (id) values (1) returning doctypes the realusers.doccolumn as{ k: number }.
Technical details
# WITH-query lookup in `derived_target` ignores scope
## Affected sites
- engine/postgres/src/jsonshape.rs:410 — `self.ctes.iter().find(|(name, _)| *name == rv.relname)` matches any unqualified RangeVar in any context
- engine/postgres/src/jsonshape.rs:128-136 — `ctes` is a flat list of the top statement's WITH queries only
- engine/postgres/src/jsonshape.rs:426-441 — `derived` pushes a context without changing which CTE names are visible
## Reproductions (temp tests at 998697d)
- `probes_ahead("WITH t AS (SELECT jsonb_build_object('n', t.o) AS o FROM t) SELECT o FROM t")` → `thread 'reylo-engine' has overflowed its stack`, SIGABRT
- `WITH x AS (SELECT jsonb_build_object('a', 1) AS o) SELECT y.v FROM (WITH x AS (SELECT '{"b": "s"}'::jsonb AS o) SELECT jsonb_build_object('inner', x.o) AS v FROM x) y` → `v: { inner: { a: … } }`
- `WITH users AS (SELECT jsonb_build_object('k', 1) AS doc) INSERT INTO users (id) VALUES (1) RETURNING doc` → `doc: { k: … }`
## Required outcome
- A name resolves to a WITH query only where Postgres would resolve it to that query:
- inside a WITH query's own body, only earlier siblings are visible (non-recursive WITH)
- an inner query's own WITH shadows outer names
- a DML statement's target relation is never a WITH query
- Otherwise the lookup returns `None`.
- Resolution can never recurse without bound.
## Suggested approach (optional)
Keep a stack of visible WITH-query scopes next to `stack`/`withs`, the way `Withs::with` already tracks names per level. Push the item body's `with_clause` in `derived` and in the SubLink arm. When walking a top-level WITH query's body, expose only the queries before it. In `top_context`, exclude the relation from `relation_from`/MERGE.There was a problem hiding this comment.
Right on all three. This PR was closed without merging and won't change; the fix is #53 (fix(postgres): scope the names json shapes read as postgres does). A non-recursive WITH query's body now sees only the queries before it, so its own name and a later one's are tables there, and nothing recurses. A subquery with a WITH of its own is not read. A DML statement whose WITH names the relation it writes gets no shapes. Your three statements, and variants of them, are in oracle/cases/json_scoping.
| }, | ||
| }; | ||
| let body = match item.node.as_ref()? { | ||
| NodeEnum::RangeSubselect(r) => r.subquery.as_deref().and_then(as_select)?.clone(), |
There was a problem hiding this comment.
The column-alias list of the FROM item is ignored here. (…) x(b, a) and w as y(b, a) rename the item's columns by position, but the target is still matched by its inner name (L416-419). As a result, select x.a from (select jsonb_build_object('k', 1) as a, '{}'::jsonb as b from users) x(b, a) is typed { k: number } while the row holds {} (reproduced; the same happens through a CTE reference). The WITH queries already bail on aliascolnames. Bailing the same way when r.alias or rv.alias has non-empty colnames, or matching by position, would keep this sound.
There was a problem hiding this comment.
Right. This PR was closed without merging and won't change. Derived tables and WITH queries are now typed by #3 on main, which renames an item's columns by its column list, position by position; #53 adds your statement and the WITH variants (alias_list, cte_alias_list, cte_reference_alias_list in oracle/cases/json_scoping). The LATERAL half is #55, which renames a lateral subquery's columns by its alias's column list in the same way, and gives no shape where two columns share a name.
60ac8cf to
00b6ff8
Compare
Stacked on #5 (its branch is this PR's base); it retargets to
mainwhen that one merges.JSON a statement builds kept its shape only through a scalar subquery: read through a subquery in
FROM, aWITHquery, aLATERALjoin or aCASE, it came back asunknown. ALATERALsubquery that aggregates a child table is the usual way to return nested rows, so this was the common case.select m.id, x.teams from member m left join lateral (select json_agg(json_build_object('id', t.id, 'name', t.name)) as teams from member_team mt join team t on t.id = mt.team_id where mt.member_id = m.id) x on trueteams: unknownteams: { id: number | null; name: string | null }[] | null… cross join lateral (select coalesce(json_agg(json_build_object('id', t.id)), '[]') as teams …) xteams: unknownteams: { id: number }[]with x as (select jsonb_build_object('n', name, 'm', motto) as o from team) select o from xo: unknowno: { n: string; m: string | null }select t.id, x.o from team t left join (select id, jsonb_build_object('n', first_name) as o from member) x on x.id = t.ido: unknowno: { n: string } | nullselect case when m.id > 1 then jsonb_build_object('n', m.first_name) else null end as o from member mo: unknowno: { n: string } | nulljsonshape.rs,derived_targetandderived):x.colof a subquery inFROM, aWITHquery or aLATERALsubquery, or a barecolof the one item aFROMholds, takes the shape of the targetcolthere, built in that item's own context (itsFROMandWHERE, and everyFROMaround it for aLATERALone's correlated columns). An outer join that can null the item makes the value nullable. An item that is a set operation or selects a*is left as it was, since a name there may be another target.CASEwhose one branch builds JSON and whose other branches areNULLtakes that branch's shape, nullable. Two branches that build JSON may build two shapes, and are left as they were.LATERALstay nullable where the probe reads a table twice; feat(postgres): infer more expressions non-null #15 tells those readings apart.New oracle case
oracle/cases/json_derived, which runs each statement over seeded edge values; no existing case changed. Docs: the Postgres page's JSON section.Verified locally: the engine's native tests, every oracle case on Postgres 18.6, SQLite and MySQL 8.4 with
--strict-coverage, the rebuiltpackages/postgres/engine.wasm,nub run typecheck, the loader's tests with every lab installed, the package build and the docs site.