From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <0AABDF21-3DB1-4344-B957-085FB5027C95@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_C5508CCD-1D21-41DD-A849-C35F6D05BB5B"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\))
Subject: Re: Fix GROUP BY ALL handling of ORDER BY operator semantics
Date: Mon, 29 Jun 2026 17:09:59 +0800
In-Reply-To: <5243308F-8E5C-45AA-828C-FAD96C4F34DA@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
References: <5243308F-8E5C-45AA-828C-FAD96C4F34DA@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/0AABDF21-3DB1-4344-B957-085FB5027C95%40gmail.com>
Precedence: bulk


--Apple-Mail=_C5508CCD-1D21-41DD-A849-C35F6D05BB5B
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Jun 29, 2026, at 15:20, Chao Li <li.evan.chao@gmail.com> wrote:
>=20
> Hi,
>=20
> While testing =E2=80=9C[ef38a4d97] Add GROUP BY ALL=E2=80=9D, 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.
>=20
> 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.
>=20
> For non-ALL, it calls transformGroupClauseExpr() for every group =
clause. Inside transformGroupClauseExpr(), there is logic that the ALL =
path misses:
> ```
> /*
> * If the GROUP BY tlist entry also appears in ORDER BY, copy operator
> * info from the (first) matching ORDER BY item.  This means that if
> * you write something like "GROUP BY foo ORDER BY foo USING <<<", the
> * GROUP BY operation silently takes on the equality semantics implied
> * by the ORDER BY.  There are two reasons to do this: it improves the
> * odds that we can implement both GROUP BY and ORDER BY with a single
> * sort step, and it allows the user to choose the equality semantics
> * used by GROUP BY, should she be working with a datatype that has
> * more than one equality operator.
> *
> * If we're in a grouping set, though, we force our requested ordering
> * to be NULLS LAST, because if we have any hope of using a sorted agg
> * for the job, we're going to be tacking on generated NULL values
> * after the corresponding groups. If the user demands nulls first,
> * another sort step is going to be inevitable, but that's the
> * planner's problem.
> */
>=20
> foreach(sl, sortClause)
> {
> SortGroupClause *sc =3D (SortGroupClause *) lfirst(sl);
>=20
> if (sc->tleSortGroupRef =3D=3D tle->ressortgroupref)
> {
> SortGroupClause *grpc =3D copyObject(sc);
>=20
> if (!toplevel)
> grpc->nulls_first =3D false;
> *flatresult =3D lappend(*flatresult, grpc);
> found =3D true;
> break;
> }
> }
> ```
>=20
> Based on this finding, I built a test using record_image_ops, where =
row(1.0) and row(1.00) compare as distinct under record-image equality:
> ```
> evantest=3D# CREATE TYPE t_rec AS (x numeric);
> CREATE TYPE
> evantest=3D#
> evantest=3D# SELECT row(1.0)::t_rec OPERATOR(pg_catalog.*=3D) =
row(1.00)::t_rec;
> ?column?
> ----------
> f
> (1 row)
> ```
>=20
> Here is the repro:
> ```
> evantest=3D# create table t (a t_rec);
> CREATE TABLE
> evantest=3D# set enable_hashagg =3D 0;
> SET
> evantest=3D# insert into t values(row(1.0)::t_rec), =
(row(1.00)::t_rec);
> INSERT 0 2
> evantest=3D# 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)
>=20
> evantest=3D# select a, count(a) from t group by all order by a using =
operator(pg_catalog.*<);
>   a   | count
> -------+-------
> (1.0) |     2
> (1 row)
> ```
>=20
> 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.
>=20
> 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.
>=20

Oops! Once again, forgot attachment.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/


--Apple-Mail=_C5508CCD-1D21-41DD-A849-C35F6D05BB5B
Content-Disposition: attachment;
	filename=v1-0001-Fix-GROUP-BY-ALL-handling-of-ORDER-BY-operator-se.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v1-0001-Fix-GROUP-BY-ALL-handling-of-ORDER-BY-operator-se.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20b6e19f5c8eea6a17a06a2d836f6f1f5a62c149c6=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Mon,=2029=20Jun=202026=2014:22:36=20+0800=0ASubject:=20[PATCH=20=
v1]=20Fix=20GROUP=20BY=20ALL=20handling=20of=20ORDER=20BY=20operator=20=
semantics=0A=0AGROUP=20BY=20ALL=20builds=20its=20grouping=20list=20=
directly=20from=20the=20query=20target=0Alist.=20In=20doing=20so,=20it=20=
used=20addTargetToGroupList()=20and=20skipped=20the=0Aexplicit=20GROUP=20=
BY=20path's=20handling=20for=20target=20entries=20that=20also=20appear=0A=
in=20ORDER=20BY.=0A=0AThat=20matters=20when=20ORDER=20BY=20...=20USING=20=
selects=20non-default=20operator=0Asemantics.=20An=20explicit=20GROUP=20=
BY=20item=20copies=20the=20matching=20ORDER=20BY=0ASortGroupClause,=20so=20=
grouping=20uses=20the=20equality=20semantics=20implied=20by=0Athe=20=
ordering=20operator.=20GROUP=20BY=20ALL=20instead=20used=20the=20default=20=
grouping=0Asemantics,=20making=20it=20not=20equivalent=20to=20spelling=20=
out=20the=20inferred=0Agrouping=20expressions.=0A=0AFix=20by=20factoring=20=
the=20"ORDER=20BY"-aware=20grouping-list=20construction=20into=20a=0A=
helper=20and=20using=20it=20for=20both=20explicit=20GROUP=20BY=20items=20=
and=20GROUP=20BY=20ALL=0Ainferred=20items.=20Add=20regression=20coverage=20=
with=20record_image_ops,=20where=0Adefault=20record=20equality=20and=20=
image=20equality=20distinguish=20row(1.0)=20and=0Arow(1.00)=20=
differently.=0A=0AAuthor:=20Chao=20Li=20<lic@highgo.com>=0A---=0A=20=
src/backend/parser/parse_clause.c=20=20=20=20=20=20=20=20|=2076=20=
+++++++++++++++++-------=0A=20src/test/regress/expected/aggregates.out=20=
|=2027=20+++++++++=0A=20src/test/regress/sql/aggregates.sql=20=20=20=20=20=
=20|=2018=20++++++=0A=203=20files=20changed,=2098=20insertions(+),=2023=20=
deletions(-)=0A=0Adiff=20--git=20a/src/backend/parser/parse_clause.c=20=
b/src/backend/parser/parse_clause.c=0Aindex=205fe5257b019..68e0049c04c=20=
100644=0A---=20a/src/backend/parser/parse_clause.c=0A+++=20=
b/src/backend/parser/parse_clause.c=0A@@=20-96,6=20+96,13=20@@=20static=20=
List=20*resolve_unique_index_expr(ParseState=20*pstate,=20InferClause=20=
*infer,=0A=20=09=09=09=09=09=09=09=09=09=20=20=20Relation=20heapRel);=0A=20=
static=20List=20*addTargetToGroupList(ParseState=20*pstate,=20=
TargetEntry=20*tle,=0A=20=09=09=09=09=09=09=09=09=20=20List=20=
*grouplist,=20List=20*targetlist,=20int=20location);=0A+static=20List=20=
*addTargetToGroupListWithSortClause(ParseState=20*pstate,=0A+=09=09=09=09=
=09=09=09=09=09=09=09=20=20=20TargetEntry=20*tle,=0A+=09=09=09=09=09=09=09=
=09=09=09=09=20=20=20List=20*grouplist,=0A+=09=09=09=09=09=09=09=09=09=09=
=09=20=20=20List=20*targetlist,=0A+=09=09=09=09=09=09=09=09=09=09=09=20=20=
=20List=20*sortClause,=0A+=09=09=09=09=09=09=09=09=09=09=09=20=20=20int=20=
location,=0A+=09=09=09=09=09=09=09=09=09=09=09=20=20=20bool=20toplevel);=0A=
=20static=20WindowClause=20*findWindowClause(List=20*wclist,=20const=20=
char=20*name);=0A=20static=20Node=20*transformFrameOffset(ParseState=20=
*pstate,=20int=20frameOptions,=0A=20=09=09=09=09=09=09=09=09=20=20Oid=20=
rangeopfamily,=20Oid=20rangeopcintype,=20Oid=20*inRangeFunc,=0A@@=20=
-2514,7=20+2521,6=20@@=20transformGroupClauseExpr(List=20**flatresult,=20=
Bitmapset=20*seen_local,=0A=20=09=09=09=09=09=09=20ParseExprKind=20=
exprKind,=20bool=20useSQL99,=20bool=20toplevel)=0A=20{=0A=20=09=
TargetEntry=20*tle;=0A-=09bool=09=09found=20=3D=20false;=0A=20=0A=20=09=
if=20(useSQL99)=0A=20=09=09tle=20=3D=20findTargetlistEntrySQL99(pstate,=20=
gexpr,=0A@@=20-2525,8=20+2531,6=20@@=20transformGroupClauseExpr(List=20=
**flatresult,=20Bitmapset=20*seen_local,=0A=20=0A=20=09if=20=
(tle->ressortgroupref=20>=200)=0A=20=09{=0A-=09=09ListCell=20=20=20*sl;=0A=
-=0A=20=09=09/*=0A=20=09=09=20*=20Eliminate=20duplicates=20(GROUP=20BY=20=
x,=20x)=20but=20only=20at=20local=20level.=0A=20=09=09=20*=20(Duplicates=20=
in=20grouping=20sets=20can=20affect=20the=20number=20of=20returned=0A@@=20=
-2537,14=20+2541,44=20@@=20transformGroupClauseExpr(List=20**flatresult,=20=
Bitmapset=20*seen_local,=0A=20=09=09=20*/=0A=20=09=09if=20=
(bms_is_member(tle->ressortgroupref,=20seen_local))=0A=20=09=09=09return=20=
0;=0A+=09}=0A+=0A+=09*flatresult=20=3D=20=
addTargetToGroupListWithSortClause(pstate,=20tle,=0A+=09=09=09=09=09=09=09=
=09=09=09=09=09=09=20*flatresult,=20*targetlist,=0A+=09=09=09=09=09=09=09=
=09=09=09=09=09=09=20sortClause,=0A+=09=09=09=09=09=09=09=09=09=09=09=09=09=
=20exprLocation(gexpr),=0A+=09=09=09=09=09=09=09=09=09=09=09=09=09=20=
toplevel);=0A+=0A+=09/*=0A+=09=20*=20_something_=20must=20have=20=
assigned=20us=20a=20sortgroupref=20by=20now...=0A+=09=20*/=0A+=0A+=09=
return=20tle->ressortgroupref;=0A+}=0A+=0A+/*=0A+=20*=20Add=20a=20=
targetlist=20entry=20to=20the=20GROUP=20BY=20list,=20copying=20matching=20=
ORDER=20BY=0A+=20*=20operator=20information=20if=20available.=0A+=20*/=0A=
+static=20List=20*=0A+addTargetToGroupListWithSortClause(ParseState=20=
*pstate,=20TargetEntry=20*tle,=0A+=09=09=09=09=09=09=09=09=20=20=20List=20=
*grouplist,=20List=20*targetlist,=0A+=09=09=09=09=09=09=09=09=20=20=20=
List=20*sortClause,=20int=20location,=0A+=09=09=09=09=09=09=09=09=20=20=20=
bool=20toplevel)=0A+{=0A+=09bool=09=09found=20=3D=20false;=0A+=0A+=09if=20=
(tle->ressortgroupref=20>=200)=0A+=09{=0A+=09=09ListCell=20=20=20*sl;=0A=20=
=0A=20=09=09/*=0A=20=09=09=20*=20If=20we're=20already=20in=20the=20flat=20=
clause=20list,=20we=20don't=20need=20to=20consider=0A=20=09=09=20*=20=
adding=20ourselves=20again.=0A=20=09=09=20*/=0A-=09=09found=20=3D=20=
targetIsInSortList(tle,=20InvalidOid,=20*flatresult);=0A+=09=09found=20=3D=
=20targetIsInSortList(tle,=20InvalidOid,=20grouplist);=0A=20=09=09if=20=
(found)=0A-=09=09=09return=20tle->ressortgroupref;=0A+=09=09=09return=20=
grouplist;=0A=20=0A=20=09=09/*=0A=20=09=09=20*=20If=20the=20GROUP=20BY=20=
tlist=20entry=20also=20appears=20in=20ORDER=20BY,=20copy=20operator=0A@@=20=
-2575,7=20+2609,7=20@@=20transformGroupClauseExpr(List=20**flatresult,=20=
Bitmapset=20*seen_local,=0A=20=0A=20=09=09=09=09if=20(!toplevel)=0A=20=09=
=09=09=09=09grpc->nulls_first=20=3D=20false;=0A-=09=09=09=09*flatresult=20=
=3D=20lappend(*flatresult,=20grpc);=0A+=09=09=09=09grouplist=20=3D=20=
lappend(grouplist,=20grpc);=0A=20=09=09=09=09found=20=3D=20true;=0A=20=09=
=09=09=09break;=0A=20=09=09=09}=0A@@=20-2587,15=20+2621,10=20@@=20=
transformGroupClauseExpr(List=20**flatresult,=20Bitmapset=20*seen_local,=0A=
=20=09=20*=20sort/group=20semantics.=0A=20=09=20*/=0A=20=09if=20(!found)=0A=
-=09=09*flatresult=20=3D=20addTargetToGroupList(pstate,=20tle,=0A-=09=09=09=
=09=09=09=09=09=09=09=20=20=20*flatresult,=20*targetlist,=0A-=09=09=09=09=
=09=09=09=09=09=09=20=20=20exprLocation(gexpr));=0A+=09=09grouplist=20=3D=20=
addTargetToGroupList(pstate,=20tle,=20grouplist,=0A+=09=09=09=09=09=09=09=
=09=09=09=20=20targetlist,=20location);=0A=20=0A-=09/*=0A-=09=20*=20=
_something_=20must=20have=20assigned=20us=20a=20sortgroupref=20by=20=
now...=0A-=09=20*/=0A-=0A-=09return=20tle->ressortgroupref;=0A+=09return=20=
grouplist;=0A=20}=0A=20=0A=20/*=0A@@=20-2822,16=20+2851,17=20@@=20=
transformGroupClause(ParseState=20*pstate,=20List=20*grouplist,=20bool=20=
groupByAll,=0A=20=09=09=09=09continue;=0A=20=0A=20=09=09=09/*=0A-=09=09=09=
=20*=20Otherwise,=20add=20the=20TLE=20to=20the=20result=20using=20=
default=20sort/group=0A-=09=09=09=20*=20semantics.=20=20We=20specify=20=
the=20parse=20location=20as=20the=20TLE's=0A-=09=09=09=20*=20location,=20=
despite=20the=20comment=20for=20addTargetToGroupList=0A-=09=09=09=20*=20=
discouraging=20that.=20=20The=20only=20other=20thing=20we=20could=20=
point=20to=20is=0A-=09=09=09=20*=20the=20ALL=20keyword,=20which=20seems=20=
unhelpful=20when=20there=20are=20multiple=0A-=09=09=09=20*=20TLEs.=0A+=09=
=09=09=20*=20Otherwise,=20add=20the=20TLE=20to=20the=20result.=20=20We=20=
specify=20the=20parse=0A+=09=09=09=20*=20location=20as=20the=20TLE's=20=
location,=20despite=20the=20comment=20for=0A+=09=09=09=20*=20=
addTargetToGroupList=20discouraging=20that.=20=20The=20only=20other=20=
thing=0A+=09=09=09=20*=20we=20could=20point=20to=20is=20the=20ALL=20=
keyword,=20which=20seems=20unhelpful=0A+=09=09=09=20*=20when=20there=20=
are=20multiple=20TLEs.=0A=20=09=09=09=20*/=0A-=09=09=09result=20=3D=20=
addTargetToGroupList(pstate,=20tle,=0A-=09=09=09=09=09=09=09=09=09=09=20=20=
result,=20*targetlist,=0A-=09=09=09=09=09=09=09=09=09=09=20=20=
exprLocation((Node=20*)=20tle->expr));=0A+=09=09=09result=20=3D=20=
addTargetToGroupListWithSortClause(pstate,=20tle,=0A+=09=09=09=09=09=09=09=
=09=09=09=09=09=09=09result,=20*targetlist,=0A+=09=09=09=09=09=09=09=09=09=
=09=09=09=09=09sortClause,=0A+=09=09=09=09=09=09=09=09=09=09=09=09=09=09=
exprLocation((Node=20*)=20tle->expr),=0A+=09=09=09=09=09=09=09=09=09=09=09=
=09=09=09true);=0A=20=09=09}=0A=20=0A=20=09=09/*=20If=20we=20found=20any=20=
acceptable=20targets,=20we're=20done=20*/=0Adiff=20--git=20=
a/src/test/regress/expected/aggregates.out=20=
b/src/test/regress/expected/aggregates.out=0Aindex=20=
89e051ee824..8863d5f9d0b=20100644=0A---=20=
a/src/test/regress/expected/aggregates.out=0A+++=20=
b/src/test/regress/expected/aggregates.out=0A@@=20-1834,6=20+1834,33=20=
@@=20SELECT=20pg_get_viewdef('v1'::regclass);=0A=20=0A=20DROP=20VIEW=20=
v1;=0A=20DROP=20TABLE=20t1;=0A+--=20GROUP=20BY=20ALL=20should=20use=20=
ORDER=20BY's=20equality=20semantics,=20just=20as=20an=0A+--=20equivalent=20=
explicit=20GROUP=20BY=20clause=20would.=0A+CREATE=20TYPE=20t_rec=20AS=20=
(x=20numeric);=0A+CREATE=20TEMP=20TABLE=20t_rec_ops=20(a=20t_rec);=0A=
+INSERT=20INTO=20t_rec_ops=20VALUES=20(row(1.0)::t_rec),=20=
(row(1.00)::t_rec);=0A+SET=20enable_hashagg=20=3D=20off;=0A+SELECT=20=
count(*)=20AS=20groups=20FROM=0A+=20=20(SELECT=20a,=20count(*)=20FROM=20=
t_rec_ops=0A+=20=20=20=20GROUP=20BY=20a=0A+=20=20=20=20ORDER=20BY=20a=20=
USING=20OPERATOR(pg_catalog.*<))=20s;=0A+=20groups=20=0A+--------=0A+=20=20=
=20=20=20=202=0A+(1=20row)=0A+=0A+SELECT=20count(*)=20AS=20groups=20FROM=0A=
+=20=20(SELECT=20a,=20count(*)=20FROM=20t_rec_ops=0A+=20=20=20=20GROUP=20=
BY=20ALL=0A+=20=20=20=20ORDER=20BY=20a=20USING=20=
OPERATOR(pg_catalog.*<))=20s;=0A+=20groups=20=0A+--------=0A+=20=20=20=20=
=20=202=0A+(1=20row)=0A+=0A+RESET=20enable_hashagg;=0A+DROP=20TABLE=20=
t_rec_ops;=0A+DROP=20TYPE=20t_rec;=0A=20--=0A=20--=20Test=20GROUP=20BY=20=
matching=20of=20join=20columns=20that=20are=20type-coerced=20due=20to=20=
USING=0A=20--=0Adiff=20--git=20a/src/test/regress/sql/aggregates.sql=20=
b/src/test/regress/sql/aggregates.sql=0Aindex=20916383db927..a2a8a4e5fa5=20=
100644=0A---=20a/src/test/regress/sql/aggregates.sql=0A+++=20=
b/src/test/regress/sql/aggregates.sql=0A@@=20-664,6=20+664,24=20@@=20=
SELECT=20pg_get_viewdef('v1'::regclass);=0A=20DROP=20VIEW=20v1;=0A=20=
DROP=20TABLE=20t1;=0A=20=0A+--=20GROUP=20BY=20ALL=20should=20use=20ORDER=20=
BY's=20equality=20semantics,=20just=20as=20an=0A+--=20equivalent=20=
explicit=20GROUP=20BY=20clause=20would.=0A+CREATE=20TYPE=20t_rec=20AS=20=
(x=20numeric);=0A+CREATE=20TEMP=20TABLE=20t_rec_ops=20(a=20t_rec);=0A=
+INSERT=20INTO=20t_rec_ops=20VALUES=20(row(1.0)::t_rec),=20=
(row(1.00)::t_rec);=0A+SET=20enable_hashagg=20=3D=20off;=0A+SELECT=20=
count(*)=20AS=20groups=20FROM=0A+=20=20(SELECT=20a,=20count(*)=20FROM=20=
t_rec_ops=0A+=20=20=20=20GROUP=20BY=20a=0A+=20=20=20=20ORDER=20BY=20a=20=
USING=20OPERATOR(pg_catalog.*<))=20s;=0A+SELECT=20count(*)=20AS=20groups=20=
FROM=0A+=20=20(SELECT=20a,=20count(*)=20FROM=20t_rec_ops=0A+=20=20=20=20=
GROUP=20BY=20ALL=0A+=20=20=20=20ORDER=20BY=20a=20USING=20=
OPERATOR(pg_catalog.*<))=20s;=0A+RESET=20enable_hashagg;=0A+DROP=20TABLE=20=
t_rec_ops;=0A+DROP=20TYPE=20t_rec;=0A+=0A=20--=0A=20--=20Test=20GROUP=20=
BY=20matching=20of=20join=20columns=20that=20are=20type-coerced=20due=20=
to=20USING=0A=20--=0A--=20=0A2.50.1=20(Apple=20Git-155)=0A=0A=

--Apple-Mail=_C5508CCD-1D21-41DD-A849-C35F6D05BB5B--