Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\))
Subject: Re: [PATCH] btree_gist: add cross-type integer operator support for
 GiST
From: Maxime Schoemans <maxime.schoemans@enterprisedb.com>
In-Reply-To: <ac05a661-53a5-478a-9907-f56a960dab51@Spark>
Date: Wed, 24 Jun 2026 12:41:37 +0200
Cc: Andrey Borodin <x4mmm@yandex-team.ru>,
 pgsql-hackers mailing list <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <68E31455-03AB-4EAA-A87C-95D95711CC70@enterprisedb.com>
References: <aac10ffa-a0ca-4c49-846b-3655cbc6b37e@Spark>
 <36b4f67d-5975-452c-a6b8-b6407f0924ee@Spark>
 <4B4B0998-7B43-4893-9603-0AF212036690@yandex-team.ru>
 <18e88767-6c31-402a-887e-37c38b366a6a@Spark>
 <80ef3b41-1a71-47a4-a320-29e118d7092c@Spark>
 <E3255EC9-4666-47FD-A22B-FDD962307FD8@enterprisedb.com>
 <ac05a661-53a5-478a-9907-f56a960dab51@Spark>
To: Alexander Nestorov <alexandernst@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/68E31455-03AB-4EAA-A87C-95D95711CC70%40enterprisedb.com>
Precedence: bulk

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