Re: GROUP BY DISTINCT
Tomas Vondra <tomas.vondra@enterprisedb.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Vik Fearing <vik@postgresfriends.org>,
Georgios Kokolatos <gkokolatos@protonmail.com>,
pgsql-hackers@lists.postgresql.org
Cc: Erik Rijkers <er@xs4all.nl>
Date: 2021-03-12T23:33:36Z
Lists: pgsql-hackers, pgsql-docs
Hi Vik, The patch seems quite ready, I have just two comments. 1) Shouldn't this add another <indexterm> for DISTINCT, somewhere in the documentation? Now the index points just to the SELECT DISTINCT part. 2) The part in gram.y that wraps/unwraps the boolean flag as an integer, in order to stash it in the group lists is rather ugly, IMHO. It forces all the places handling the list to be aware of this (there are not many, but still ...). And there are no other places doing (bool) intVal so it's not like there's a precedent for this. I think the clean solution is to make group_clause produce a struct with two fields, and just use that. Not sure how invasive that will be outside gram.y, though. Also, the all_or_distinct vs. distinct_or_all seems a bit error-prone. I wonder if we can come up with some clearer names, describing the context of those types. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Commits
-
Use lfirst_int in cmp_list_len_contents_asc
- b822ae13ea93 14.0 landed
-
Fix misuse of foreach_delete_current().
- 1d581ce7129d 14.0 landed
-
Implement GROUP BY DISTINCT
- be45be9c33a8 14.0 landed