On 30.01.25 04:33, Mark Dilger wrote:
The previously committed patch (v19.1) already changed the gist strategy number
mapping to use the (Row)CompareType, as in Mark's proposal.
In this patch set, I'm attaching the existing standalone gist translate
function as the index AM API function for the gist index AM. And then all
existing callers are changed to call through the index AM API functions
provided by Mark's patch set.
Patches 0001, 0002, 0003 are some preparatory renaming and refactoring patches.
These all look sensible. I like that they reduce code duplication. No
objection.
Patches 0004 and 0005 are patch v19-0008 from Mark's (via Andrew) v19 patch
set, split into two patches, and with some function renaming from my side.
0004 needs the copyright notice updated to 2025.
Patch 0006 then pulls it all together. The key change is that we also need to pass the
opclass to the index AM API functions, so that access methods like gist can use it.
Actually, I changed that to pass opfamily and opcintype instead. I think this matches
better with the rest of the "Index AM API cleanup" patch set, because it's more
common to have the opfamily and type handy than the opclass. (And internally, the gist
support function is attached to the opfamily anyway, so it's actually simpler that way.)
I think this fits together quite well now. Several places where gist was
hardcoded are now fully (or mostly) independent of gist. Also, the somewhat
hackish get_equal_strategy_number() in the logical replication code disappears
completely and is replaced by a proper index AM API function. (This also takes
care of patch v19-0011 from Mark's patch set.)
I find the hardcoded GIST_AM_OID in GetOperatorFromCompareType() antithetical
to the purpose of this patch-set.
Yeah, actually this turned out to be unnecessary, because you can easily
obtain the AM OID from the passed-in opclass ID. So I have fixed it
that way. I have committed this whole patch set now with the mentioned
adjustments. I'll post a rebased version of the main patch set soon.
