Re: [PATCH] btree_gist: add cross-type integer operator support for GiST

From: Alexander Nestorov <alexandernst(at)gmail(dot)com>
To: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
Cc: pgsql-hackers mailing list <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] btree_gist: add cross-type integer operator support for GiST
Date: 2026-06-06 17:37:51
Message-ID: 18e88767-6c31-402a-887e-37c38b366a6a@Spark
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hey Andrey!

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

1)

Oh, good catch! Since the other points require some changes in the patch, 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 "foundation"
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!

3)

I was also worried about this. I wrote some checks against a hardcoded list in
the int_crosstype.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.  It would be
> nice to provide such a check someday

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

As an alternative, maybe I could use the regression suite? I can put the
knowledge of supported dispatch entries in the code, expose it as a validator
and assert it in the tests. Running "make check" would assert that these entries
agree with pg_amop in both directions.

Concretely: the integer consistent/distance functions would dispatch through 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 against the
cross-type rows in pg_amop for gist_int{2,4,8}_ops in both directions. 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? Or do you think it might be
a better idea to find a way to do it with amvalidate()?

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 version 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!

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Scott Ray 2026-06-06 18:40:31 Re: Report oldest xmin source when autovacuum cannot remove tuples
Previous Message 신성준 2026-06-06 16:52:34 Re: Add wait events for server logging destination writes