Re: GROUP BY DISTINCT

From: Georgios Kokolatos <gkokolatos(at)protonmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Vik Fearing <vik(at)postgresfriends(dot)org>
Subject: Re: GROUP BY DISTINCT
Date: 2021-03-02 15:06:28
Message-ID: 161469758801.29967.4455067072614569957.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-docs pgsql-hackers

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.

In response to

Responses

Browse pgsql-docs by date

  From Date Subject
Next Message Vik Fearing 2021-03-02 16:51:52 Re: GROUP BY DISTINCT
Previous Message Pantelis Theodosiou 2021-03-01 15:29:02 Re: missing schema_name

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2021-03-02 15:09:18 Re: [PATCH] psql : Improve code for help option
Previous Message Mark Dilger 2021-03-02 14:59:52 Re: [PATCH] regexp_positions ( string text, pattern text, flags text ) → setof int4range[]