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 <[email protected]> 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
