Re: Fix GROUP BY ALL handling of ORDER BY operator semantics
Miłosz Bieniek <bieniek.milosz@proton.me>
From: Miłosz Bieniek <bieniek.milosz@proton.me>
To: Chao Li <li.evan.chao@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>,
Tom Lane <tgl@sss.pgh.pa.us>
Date: 2026-07-07T20:00:26Z
Lists: pgsql-hackers
Hello, sorry Chao Li for spam, but I forgot to add pgsql-hackers to CC in the previous response. On Sunday, 5 July 2026 at 11:43, Chao Li <li.evan.chao@gmail.com> wrote: > Hi, > > While testing “[ef38a4d97] Add GROUP BY ALL”, I traced the code and found a suspicious difference. In theory, GROUP BY ALL should behave the same as spelling out all inferred grouping expressions, but I found that they go through different code paths. > > After entering transformGroupClause(), if this is GROUP BY ALL, it immediately enters a separate branch, loops over the target list, calls addTargetToGroupList() for every TLE, and then returns. > > For non-ALL, it calls transformGroupClauseExpr() for every group clause. Inside transformGroupClauseExpr(), there is logic that the ALL path misses: I can confirm that this path is skipped for GROUP BY ALL. > > Here is the repro: > ``` > evantest=# create table t (a t_rec); > CREATE TABLE > evantest=# set enable_hashagg = 0; > SET > evantest=# insert into t values(row(1.0)::t_rec), (row(1.00)::t_rec); > INSERT 0 2 > evantest=# select a, count(a) from t group by a order by a using operator(pg_catalog.*<); > a | count > --------+------- > (1.00) | 1 > (1.0) | 1 > (2 rows) > > evantest=# select a, count(a) from t group by all order by a using operator(pg_catalog.*<); > a | count > -------+------- > (1.0) | 2 > (1 row) > ``` > Reproduced locally, both cases now return consistent results with the patch applied. > As we can see, "GROUP BY a" distinguishes the two rows because it uses the equality semantics implied by ORDER BY ... USING, but "GROUP BY ALL" groups the two rows together because it uses the default grouping semantics instead. > > The fix mostly refactors the existing logic so the GROUP BY ALL path also handles the ORDER BY sort clause. See the attached patch for details. The patch looks correct to me. Best regards, Miłosz Bieniek