public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andrey M. Borodin <[email protected]>
To: Bernd Helmle <[email protected]>
Cc: jian he <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Add sortsupport for range types and btree_gist
Date: Mon, 11 Nov 2024 23:03:36 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CACJufxHvXHNi_9roSo2gLJVCSep1EgD-z=-ey6K_Xg96PHs9+g@mail.gmail.com>
	<CACJufxEVYNCQJEnRr6jpdrNSb9hH6YkavxrLabrRuCWoeTDHjQ@mail.gmail.com>
	<[email protected]>
	<CACJufxHSsRk9egTjZWYXK4mJF8rsoV0XuRuw4uCxymoeeYBAJw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>



> On 11 Nov 2024, at 21:41, Bernd Helmle <[email protected]> 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.

view thread (23+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Add sortsupport for range types and btree_gist
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox