Repository navigation
fix(postgres): count the other built-in aggregates as one row - #71
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
I reviewed the one commit that widens what the Postgres engine treats as an ungrouped aggregate, plus its oracle coverage and the rebuilt artifact.
- More aggregates count in
Reader::aggregates:array_agg,bit_and,bit_or,bool_and,bool_orandeverywith one argument, andstring_agg,json_object_aggandjsonb_object_aggwith two, now count on Postgres only. This affects the statement's cardinality (cardinality.rs, throughinfer::aggregates) and whether a scalar subquery over one of them can be non-null (Reader::sublink). - Shadowing still applies: every new name is already in
postgres_function_names()throughPOSTGRES_GROUPED,POSTGRES_GROUPED_NEVER_NULLandPOSTGRES_GROUPED_JSON. So a same-named function defined by a user or an extension still shadows it throughtrust.builtin. - No false
onefrom set-returning wrappers: on the cardinality path,calls_at_this_level(...).set_returningrejects a statement likeselect unnest(array_agg(id)) from postsbefore it looks for an aggregate. On the subquery path,one_valuedoes the same job. An aggregate over only outer columns is still left toLevel::owns. - Oracle: the new
aggregate_namescase covers each aggregate over seeded rows and over an always-empty table. It also covers a grouped query and scalar subqueries with and withoutcoalesce. Two existingexpected.txtlines go frommanytoone, and both are correct. - Docs:
postgres.mdxand03-inference.mdxalready say "an aggregate without agroup by" returns one row, so this change brings the engine in line with them and no docs edit is needed.
claude-opus-5-5 ｜ 𝕏
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, an aggregate without a
GROUP BYreturns exactly one row, which types the statement's cardinality asoneand a scalar subquery over it as one value. The engine counted onlycount,sum,avg,min,max,json_aggandjsonb_aggas aggregates there, so a query over any other built-in aggregate was typedmany, and a scalar subquery over one stayed nullable under acoalescewith a non-null default:select string_agg(title, ', ') as titles from postsmanyoneselect array_agg(id) as ids from postsmanyoneselect bool_and(title <> '') as all_titled, bool_or(user_id = 1) as any_first, every(id > 0) as all_positive from postsmanyoneselect (select coalesce(string_agg(p.title, ', '), '') from posts p where p.user_id = u.id) as titles from users utitles: string | nulltitles: stringselect (select coalesce(array_agg(p.id), '{}') from posts p where p.user_id = u.id) as ids from users uids: (bigint | null)[] | nullids: (bigint | null)[]select (select string_agg(p.title, ', ') from posts p where p.user_id = u.id) as titles from users utitles: string | nulltitles: string | nullNow the aggregates the engine's rules already read are counted too, each at its one arity:
array_agg,bit_and,bit_or,bool_and,bool_orandeveryof one argument, andstring_agg,json_object_aggandjsonb_object_aggof two. Each is a name the trust query already asks the catalog about, so one a user or an extension has also defined is still not taken for the built-in.Tests:
oracle/cases/aggregate_names: each aggregate without aGROUP BY, over rows and over a table that never holds one, and in scalar subqueries with and without acoalesce, sound over the seeded edge values.one, as they should be:param_keys/aggregate_order.sql(string_agg(… order by :o::text)) andprecision_rules/empty_array.sql(coalesce(array_agg(id), array[]::int[])).--strict-coverage, the engine's tests, the workspace build with no warning, the rebuilt artifact's fingerprint, the loader's typecheck and tests, the Postgres labs and the docs site.