Repository navigation
fix(postgres): type an aggregate beside a column in built json - #45
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Important
A GROUP BY that names an output column is copied as-is into the new grouped probe. That probe has no such column, so it fails, and fields main typed now come out unknown.
Reviewed changes
I reviewed the split of a grouping or aggregating statement's top JSON probe into a probe that carries the GROUP BY and one that does not.
- Two probes for the top context: non-column expressions outside an aggregate's arguments now go in a probe written with the statement's
GROUP BY(Draft.grouped,outside_aggregates). Bare columns andjsonb_agg(…)arguments stay in a probe without it. - Ungrouped aggregating statements:
aggregating/constructs_aggregatefind these. They get an emptygroup, so their aggregates are probed apart from the aggregated arguments. GROUP BYby position: this is detected by anAConstitem and falls back to the old single probe.- Tests and docs: there is a new
json_grouped_valuesoracle case with 12 statements, a unit test of the probe texts, a docs paragraph, and a rebuiltengine.wasm.
claude-opus-5-5 ｜ 𝕏
colinhacks
force-pushed
the
json-aggregate-probe
branch
from
October 22, 2026 23:40
7ebd0bb to
c0dfb4c
Compare
Contributor
There was a problem hiding this comment.
✅ No new issues found. The output-name
GROUP BYregression is fixed, so a statement like that types no worse than onmain.
Reviewed changes
I reviewed c0dfb4c, which keeps a GROUP BY that may name a select-list column out of the grouped JSON probe.
- Added
names_output: aGROUP BYitem that is a position, or a bare name equal to a top target's alias or derived name (target_name), now setsgroup = None. That falls back to the single probe from before. A target that is a bare column has no derived name, sop.title … GROUP BY titlestill carries itsGROUP BY, which the new unit assert confirms. - Extended coverage: there are two new oracle statements.
group_by_alias.sqlgives{ n: int8, best: text? }, the same as before this PR.group_by_alias_beside.sqlgivesunknown?values, the same as the positional case. There are also three unit asserts for the probe texts. - Updated docs and setup comment: the
postgres.mdxparagraph now names both forms and says the values areunknown, not the object. - Rebuilt
packages/postgres/engine.wasmin the same commit.
claude-opus-5-5 ｜ 𝕏
colinhacks
force-pushed
the
json-aggregate-probe
branch
from
October 22, 2026 23:57
c0dfb4c to
f1e1e1a
Compare
A JSON constructor's values were probed without the statement's GROUP BY, so an aggregate beside one of its columns failed the probe and left the whole object unknown. Expressions outside an aggregate's arguments are probed under the GROUP BY now; bare columns and arguments are not. A GROUP BY by position or by a name of the select list is not carried.
colinhacks
force-pushed
the
json-aggregate-probe
branch
from
October 23, 2026 00:22
f1e1e1a to
2e90fa3
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.
On Postgres, JSON a statement builds is typed by describing a probe that selects the constructor's values over the statement's
FROM. The probe had noGROUP BY, so an aggregate beside one of the statement's own columns made it fail, and every field of the object came outunknown:was
{ author: { name: unknown; posts: unknown } }and is now{ author: { name: string; posts: number } }.In a statement that groups or aggregates, every expression of its select list outside an aggregate's arguments is valid under its
GROUP BY. Now those expressions go in a probe that carries it, while the statement's bare columns and the arguments of its aggregates (whatjsonb_agg(…)collects) stay in the probe without one. That covers aggregates the engine has no rule for too, such asarray_aggor a user's own. The probe under theGROUP BYalso settles each aggregate's nullability the way the statement's own select list would:max(p.title)over an inner join is non-null, andsumof a nullable column stays nullable. A statement that neither groups nor aggregates keeps its one probe. AGROUP BYby position names the probe's own columns, so it is not carried, and such an object staysunknownas before.Tests:
oracle/cases/json_grouped_values: 12 statements (counts, sums,maxover inner and outer joins,array_agg, an expression beside an aggregate, an aggregating statement withoutGROUP BY,GROUP BYby position), sound over the seeded edge values.--strict-coverage, the loader's typecheck and tests, and the package build.