From: Georgios Kokolatos <gkokolatos@protonmail.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Vik Fearing <vik@postgresfriends.org>
Subject: Re: GROUP BY DISTINCT
Date: Tue, 02 Mar 2021 15:06:28 +0000
Message-ID: <161469758801.29967.4455067072614569957.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <c350ff6b-d70d-9952-be23-a18855b62589@postgresfriends.org>
References: <bf3805a8-d7d1-ae61-fece-761b7ff41ecc@postgresfriends.org>
<c350ff6b-d70d-9952-be23-a18855b62589@postgresfriends.org>
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
Hi,
this is a useful feature, thank you for implementing. I gather that it follows the standard, if so,
then there are definitely no objections from me.
The patch in version 2, applies cleanly and passes all the tests.
It contains documentation which seems correct to a non native speaker.
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.
Overall :+1: from me.
I will be bumping to 'Ready for Committer' unless objections.
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: gkokolatos@protonmail.com, pgsql-hackers@lists.postgresql.org, vik@postgresfriends.org
Subject: Re: GROUP BY DISTINCT
In-Reply-To: <161469758801.29967.4455067072614569957.pgcf@coridan.postgresql.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