Repository navigation
fix(postgres): own a subquery's aggregate of a whole row by bare name - #76
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Important
The rule is sound for the user's own statement, but infer::plan also refines JSON-shape leaf probes. A probe's text leaves out the outer FROMs, so the new rule can type as non-null a value Postgres returns as NULL. This is a regression from main: I reproduced it with the oracle (details inline).
Reviewed changes
I reviewed the single commit, which lets a scalar subquery own an aggregate of a whole row named by a bare name, when every FROM in scope is known not to have a column of that name.
Level::whole_row/Level::within:ownsnow accepts a bareqwhen an inner item goes byqand no FROM, inside or around, shows a columnq. Withoutoutside, it keeps the old answer.- Expression reader scope:
Reader.scopestarts from the text's top FROM inplan_hereand grows by oneLevelfor each subquery it enters. - JSON-shape scope: the SubLink arm builds
outsidefrom the builder's wholestack. - Oracle:
aggregate_levels/whole_row_owngoes fromint8?toint8, and there are nine newjson_subqueriesstatements.engine.wasmis rebuilt.
On a local Postgres 16, the PR's json_subqueries and aggregate_levels cases and the levels unit tests pass.
claude-opus-5-5 ｜ 𝕏
00a4e64 to
b56c77d
Compare
There was a problem hiding this comment.
✅ No new issues found. The leaf-probe regression from the last review is closed by the rebase onto #78.
Reviewed changes
I reviewed the rebase of the single commit onto #78. The engine and oracle diff is the same as before. The only text that changed is the json_subqueries/setup.sql header comment, now merged with #78's.
- Leaf-probe scope, through #78:
Builder::renamed_in_probenow sends a leaf to the widened probe when one of its bare names (subqueries included) might be a column of a FROM around it. So the top FROM thatplan_hereseedsReader.scopewith holds that outer column, andLevel::whole_rowdeclines. When the narrow probe is still used, each bare name either is a column of the current FROM, which failslacksin the probe too, or is certainly absent from every outer FROM. Either way it resolves in the probe as it does in the statement. - Guard:
json_subqueries/field_outer_column_in_subquery.sqlis the earlier repro, written against this case's schema. It staysn: int8?on this branch.
claude-opus-5-5 ｜ 𝕏
b56c77d to
c9b8999
Compare
c9b8999 to
24224c6
Compare
A scalar subquery that aggregates a whole row by its bare name came back nullable, even though an aggregate without a
group byalways returns one row. Wrapping it incoalesce(…, '[]')didn't help either:orders: ({ id: number; user_id: number; note: string | null })[] | nullorders: ({ id: number; user_id: number; note: string | null })[]coalesce(jsonb_agg(o.id), '[]')andjsonb_agg(o order by o.id)were already right, because a qualified column says which query the aggregate belongs to.The rule
An aggregate belongs to the innermost query whose columns it reads. A bare name means a column of any FROM in scope before it means an item's whole row, so the check that decides which query owns an aggregate used to treat a bare
oas possibly the outer query's. Nowjsonb_agg(o)counts as the subquery's own when all three of these hold:o;o;o, and the caller knows every FROM around it.Where the caller doesn't know the FROMs around the subquery, the old answer stands. Both readers pass in what is around: the JSON shapes, from their stack of FROMs, and the expression reader, from the top FROM and each subquery it enters.
Oracle
aggregate_levels/whole_row_own:(select count(posts) from posts where false):n: int8?→n: int8json_subqueries, each checked over seeded rows:coalesce(jsonb_agg(q), '[]'), plain, with'[]'::jsonband withjson_aggcoalesce(array_agg(q), '{}')(the array is now non-null; its element fields are unchanged)jsonb_agg(q)stays nullable, pluscoalesce(jsonb_agg(q.id), '[]'),count(*)and the top-levelcoalesce(jsonb_agg(q), '[]')