From: 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: Tue, 2 Mar 2021 17:51:52 +0100
Message-ID: <4294e82f-a134-5dbb-d9d6-03d72dce2e1e@postgresfriends.org> (raw)
In-Reply-To: <161469758801.29967.4455067072614569957.pgcf@coridan.postgresql.org>
References: <bf3805a8-d7d1-ae61-fece-761b7ff41ecc@postgresfriends.org>
<c350ff6b-d70d-9952-be23-a18855b62589@postgresfriends.org>
<161469758801.29967.4455067072614569957.pgcf@coridan.postgresql.org>
On 3/2/21 4:06 PM, Georgios Kokolatos wrote:
> As a minor gripe, I would note the addition of list_int_cmp.
> The block
>
> + /* Sort each groupset individually */
> + foreach(cell, result)
> + list_sort(lfirst(cell), list_int_cmp);
>
> Can follow suit from the rest of the code, and define a static cmp_list_int_asc(), as
> indeed the same patch does for cmp_list_len_contents_asc.
> This is indeed point of which I will not hold a too strong opinion.
I did it this way because list_int_cmp is a general purpose function for
int lists that can be reused elsewhere in the future. Whereas
cmp_list_len_contents_asc is very specific to this case so I kept it local.
I'm happy to change it around if that's what consensus wants.
> Overall :+1: from me.
Thanks for looking at it!
> I will be bumping to 'Ready for Committer' unless objections.
In that case, here is another patch that fixes a typo in the docs
mentioned privately to me by Erik. The typo (and a gratuitous rebase)
is the only change to what you just reviewed.
--
Vik Fearing
Attachments:
[text/x-patch] 0001-implement-GROUP-BY-DISTINCT.v03.patch (0B, ../4294e82f-a134-5dbb-d9d6-03d72dce2e1e@postgresfriends.org/2-0001-implement-GROUP-BY-DISTINCT.v03.patch)
download
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: vik@postgresfriends.org, gkokolatos@protonmail.com, pgsql-hackers@lists.postgresql.org, er@xs4all.nl
Subject: Re: GROUP BY DISTINCT
In-Reply-To: <4294e82f-a134-5dbb-d9d6-03d72dce2e1e@postgresfriends.org>
* 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