public inbox for [email protected]
help / color / mirror / Atom feedFrom: Maxime Schoemans <[email protected]>
To: Alexander Nestorov <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: pgsql-hackers mailing list <[email protected]>
Subject: Re: [PATCH] btree_gist: add cross-type integer operator support for GiST
Date: Wed, 24 Jun 2026 12:41:37 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <ac05a661-53a5-478a-9907-f56a960dab51@Spark>
References: <aac10ffa-a0ca-4c49-846b-3655cbc6b37e@Spark>
<36b4f67d-5975-452c-a6b8-b6407f0924ee@Spark>
<[email protected]>
<18e88767-6c31-402a-887e-37c38b366a6a@Spark>
<80ef3b41-1a71-47a4-a320-29e118d7092c@Spark>
<[email protected]>
<ac05a661-53a5-478a-9907-f56a960dab51@Spark>
Hi Alexander,
> I think we still need to check `InvalidOid` because execIndexing.c:~800
> initializes scan keys ('ScanKeyEntryInitialize') with `InvalidOid` as the
> subtype. Running the 'not_equal', 'partitions' and 'without_overlaps' tests
> confirms that 'InvalidOid' must be handled. I can add the check again in the
> switch. (attaching patch)
You are completely right, I missed this while reading the source code.
> I completely agree, your patch makes it much cleaner.
> I applied your patch (with the 'InvalidOid' fix) on top of mine and ran all the
> tests and benchmarks that I prepared previously. [...]
I'm glad you like the suggestion, and thank you for the pgindent
fixes that I missed.
Since this implementation now replaces the initial one in 0001, it
might be worth squashing 0001 and 0004 (and the 0005 fixes) into a
single 0001 patch, unless you want to keep the history of this work
in the patch series.
> I added one more commit that adds a small description of this fix in the docs.
This patch also looks good to me, although I may not be the best
person to review doc changes.
One last tiny thing: 0001 adds int_crosstype to the regress tests
but the test file is only added in 0002, so 0001 on its own fails
`make check`. Might be worth moving the Makefile/meson.build
registration into 0002 so each patch passes its own tests.
I think that one additional thing needed before it's ready would be
to add commit messages to each patch.
Other than that it looks good, I don't have any additional code
comments.
Best,
Maxime
view thread (9+ 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]
Subject: Re: [PATCH] btree_gist: add cross-type integer operator support for 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