From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Vik Fearing <vik@postgresfriends.org>
To: Georgios Kokolatos <gkokolatos@protonmail.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Erik Rijkers <er@xs4all.nl>
Subject: Re: GROUP BY DISTINCT
Date: Sat, 13 Mar 2021 00:33:36 +0100
Message-ID: <b95ee556-c527-04b8-6cae-f55950dde94b@enterprisedb.com> (raw)
In-Reply-To: <4294e82f-a134-5dbb-d9d6-03d72dce2e1e@postgresfriends.org>
References: <bf3805a8-d7d1-ae61-fece-761b7ff41ecc@postgresfriends.org>
<c350ff6b-d70d-9952-be23-a18855b62589@postgresfriends.org>
<161469758801.29967.4455067072614569957.pgcf@coridan.postgresql.org>
<4294e82f-a134-5dbb-d9d6-03d72dce2e1e@postgresfriends.org>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-docs@postgresql.org
Cc: tomas.vondra@enterprisedb.com, vik@postgresfriends.org, gkokolatos@protonmail.com, pgsql-hackers@lists.postgresql.org, er@xs4all.nl
Subject: Re: GROUP BY DISTINCT
In-Reply-To: <b95ee556-c527-04b8-6cae-f55950dde94b@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox