Date: Sat, 6 Jun 2026 19:37:51 +0200
From: Alexander Nestorov <alexandernst@gmail.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: pgsql-hackers mailing list <pgsql-hackers@postgresql.org>
Message-ID: <18e88767-6c31-402a-887e-37c38b366a6a@Spark>
In-Reply-To: <4B4B0998-7B43-4893-9603-0AF212036690@yandex-team.ru>
References: <aac10ffa-a0ca-4c49-846b-3655cbc6b37e@Spark>
 <36b4f67d-5975-452c-a6b8-b6407f0924ee@Spark>
 <4B4B0998-7B43-4893-9603-0AF212036690@yandex-team.ru>
Subject: Re: [PATCH] btree_gist: add cross-type integer operator support
 for GiST
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary="6a245af4_41b71efb_76fe"
Archived-At: 
 <https://www.postgresql.org/message-id/18e88767-6c31-402a-887e-37c38b366a6a%40Spark>
Precedence: bulk

--6a245af4_41b71efb_76fe
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

Hey Andrey=21

I'm happy to hear you like the proposal=21
Let me reply in order:

1)

Oh, good catch=21 Since the other points require some changes in the patc=
h, I'll
first do them and then I'll make sure to create a benchmark and post the
results.

2)

Agreed, that's a fair point. I was speculating with a generalized =22foun=
dation=22
because I thought I'd get questions about how I'd support other types if =
I went
with a simpler patch that covered only ints.

Let me rework that and send a new patch=21

3)

I was also worried about this. I wrote some checks against a hardcoded li=
st in
the int=5Fcrosstype.sql (which I didn't submit, you noticed), but that is=
 not
checking anything besides the SQL part.

I checked amvalidate(), but I couldn't find a way to check if there are
inconsistencies between what is defined in C and what is defined in SQL.
That function only checks amproc/amop signatures and I wasn't able to see=
 how it
could compare against the C dispatch.
There is a comment in opclasscmds.c, around line 400, that already hints =
about
it not being able to run such validation.

> we have no way to validate that the offered set of operators and
> functions are consistent with the AM's expectations.=C2=A0=C2=A0It woul=
d be
> nice to provide such a check someday

I could create a new core hook, but maybe that would be a larger change t=
hat
will expand the scope of this patch, and since neither CREATE nor ALTER A=
DD
calls it, the hook would still need a test to fire it.

As an alternative, maybe I could use the regression suite=3F I can put th=
e
knowledge of supported dispatch entries in the code, expose it as a valid=
ator
and assert it in the tests. Running =22make check=22 would assert that th=
ese entries
agree with pg=5Famop in both directions.

Concretely: the integer consistent/distance functions would dispatch thro=
ugh a
small static table of supported subtype OIDs. A set-returning C function =
would
expose that same table to SQL, and a regression test would EXCEPT it agai=
nst the
cross-type rows in pg=5Famop for gist=5Fint=7B2,4,8=7D=5Fops in both dire=
ctions. Adding an
amop without a matching dispatch entry (or vice versa) would show up as a=
 diff
under make check.

Would you prefer the regression suite approach=3F Or do you think it migh=
t be
a better idea to find a way to do it with amvalidate()=3F

4)

All right, I'll make sure to add it to the next patch.

To sum up, agreeing on 3) means I'd be ready for working on the next vers=
ion of
the patch. I just need your advice on that bit.

I was not aware of the existence of commitfest, let me register :)

Thank you very much Andrey, best regards=21

--6a245af4_41b71efb_76fe
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

<html xmlns=3D=22http://www.w3.org/1999/xhtml=22>
<head>
<title></title>
</head>
<body>
<div name=3D=22messageBodySection=22>
<div dir=3D=22auto=22>Hey Andrey=21<br />
<br />
I'm happy to hear you like the proposal=21<br />
Let me reply in order:<br />
<br />
1)<br />
<br />
Oh, good catch=21 Since the other points require some changes in the patc=
h, I'll<br />
first do them and then I'll make sure to create a benchmark and post the<=
br />
results.<br />
<br />
2)<br />
<br />
Agreed, that's a fair point. I was speculating with a generalized =22foun=
dation=22<br />
because I thought I'd get questions about how I'd support other types if =
I went<br />
with a simpler patch that covered only ints.<br />
<br />
Let me rework that and send a new patch=21<br />
<br />
3)<br />
<br />
I was also worried about this. I wrote some checks against a hardcoded li=
st in<br />
the int=5Fcrosstype.sql (which I didn't submit, you noticed), but that is=
 not<br />
checking anything besides the SQL part.<br />
<br />
I checked amvalidate(), but I couldn't find a way to check if there are<b=
r />
inconsistencies between what is defined in C and what is defined in SQL.<=
br />
That function only checks amproc/amop signatures and I wasn't able to see=
 how it<br />
could compare against the C dispatch.<br />
There is a comment in opclasscmds.c, around line 400, that already hints =
about<br />
it not being able to run such validation.<br />
<br />
&gt; we have no way to validate that the offered set of operators and<br =
/>
&gt; functions are consistent with the AM's expectations.&=23160;&=23160;=
It would be<br />
&gt; nice to provide such a check someday<br />
<br />
I could create a new core hook, but maybe that would be a larger change t=
hat<br />
will expand the scope of this patch, and since neither CREATE nor ALTER A=
DD<br />
calls it, the hook would still need a test to fire it.<br />
<br />
As an alternative, maybe I could use the regression suite=3F I can put th=
e<br />
knowledge of supported dispatch entries in the code, expose it as a valid=
ator<br />
and assert it in the tests. Running =22make check=22 would assert that th=
ese entries<br />
agree with pg=5Famop in both directions.<br />
<br />
Concretely: the integer consistent/distance functions would dispatch thro=
ugh a<br />
small static table of supported subtype OIDs. A set-returning C function =
would<br />
expose that same table to SQL, and a regression test would EXCEPT it agai=
nst the<br />
cross-type rows in pg=5Famop for gist=5Fint=7B2,4,8=7D=5Fops in both dire=
ctions. Adding an<br />
amop without a matching dispatch entry (or vice versa) would show up as a=
 diff<br />
under make check.<br />
<br />
Would you prefer the regression suite approach=3F Or do you think it migh=
t be<br />
a better idea to find a way to do it with amvalidate()=3F<br />
<br />
4)<br />
<br />
All right, I'll make sure to add it to the next patch.<br />
<br />
To sum up, agreeing on 3) means I'd be ready for working on the next vers=
ion of<br />
the patch. I just need your advice on that bit.<br />
<br />
I was not aware of the existence of commitfest, let me register :)<br />
<br />
Thank you very much Andrey, best regards=21<br /></div>
</div>
</body>
</html>

--6a245af4_41b71efb_76fe--