Re: [PATCH] Add sortsupport for range types and btree_gist - Mailing list pgsql-hackers

> On 11 Nov 2024, at 21:41, Bernd Helmle <mailings@oopsware.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.