From: | "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru> |
---|---|
To: | Bernd Helmle <mailings(at)oopsware(dot)de> |
Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] Add sortsupport for range types and btree_gist |
Date: | 2024-11-11 18:03:36 |
Message-ID: | 7095B5FA-FADB-4524-B858-ECD14998710B@yandex-team.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 11 Nov 2024, at 21:41, Bernd Helmle <mailings(at)oopsware(dot)de> wrote:
>
> Updated and rebased patches attached.
Hi Bernd!
Some nitpicking:
0. postgres % git apply ~/Downloads/v7.3-Add-GIST-sortsupport-*
/Users/x4mmm/Downloads/v7.3-Add-GIST-sortsupport-btree-gist.patch:19: space before tab in indent.
oid oid_buffering timestamp timestamp_buffering timestamptz \
/Users/x4mmm/Downloads/v7.3-Add-GIST-sortsupport-btree-gist.patch:20: space before tab in indent.
timestamptz_buffering time time_buffering timetz timetz_buffering date \
/Users/x4mmm/Downloads/v7.3-Add-GIST-sortsupport-btree-gist.patch:21: space before tab in indent.
date_buffering interval interval_buffering macaddr macaddr_buffering \
/Users/x4mmm/Downloads/v7.3-Add-GIST-sortsupport-btree-gist.patch:22: space before tab in indent.
macaddr8 macaddr8_buffering inet inet_buffering cidr cidr_buffering text \
/Users/x4mmm/Downloads/v7.3-Add-GIST-sortsupport-btree-gist.patch:23: space before tab in indent.
text_buffering varchar varchar_buffering char char_buffering \
warning: 5 lines add whitespace errors.
1. I'm mostly seening patches formatted with `git patch-format` rather than diffs as patches...
2. Changes in contrib/btree_gist/btree_gist.c seem unnecessary.
3. Why do you move "typedef struct int32key" to btree_gist.h, but do not need to move all other keys?
4. These ifdedfs are not needed, just do INJECTION_POINT()
#ifdef USE_INJECTION_POINTS
INJECTION_POINT("btree-gist-sorted-build");
#endif
Also, there are 61 occurance of this code. Why not just 1 in gist_indexsortbuild() ?
Thanks!
Best regards, Andrey Borodin.
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2024-11-11 18:15:17 | Re: Parallel heap vacuum |
Previous Message | Peter Geoghegan | 2024-11-11 18:03:07 | Re: index prefetching |