From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
---|---|
To: | Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Alex Wang <alex(dot)wang(at)enterprisedb(dot)com>, Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> |
Subject: | Re: Index AM API cleanup |
Date: | 2025-02-03 11:08:52 |
Message-ID: | b5ed0b7a-8cb9-4564-9289-ffeb5bf685e3@eisentraut.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2025-02-03 11:17:27 | Re: Optimize scram_SaltedPassword performance |
Previous Message | Dmitry Koterov | 2025-02-03 11:04:03 | Re: Increased work_mem for "logical replication tablesync worker" only? |