Re: GROUP BY DISTINCT

Georgios <gkokolatos@protonmail.com>

From: Georgios Kokolatos <gkokolatos@protonmail.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Vik Fearing <vik@postgresfriends.org>
Date: 2021-03-02T15:06:28Z
Lists: pgsql-hackers, pgsql-docs
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.

Commits

  1. Use lfirst_int in cmp_list_len_contents_asc

  2. Fix misuse of foreach_delete_current().

  3. Implement GROUP BY DISTINCT