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.
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 |
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[] |