Re: Index AM API cleanup - Mailing list pgsql-hackers

On Thu, 31 Oct 2024 at 15:02, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>
>
>
> > On Oct 30, 2024, at 12:54 PM, Peter Eisentraut <peter@eisentraut.org> wrote:
> >
> > So this is the idea.  To take a step back, I can see the following
> > options:
> >
> > 1. Require all index AMs to use btree-compatible strategy numbers.
> >   (Previously rejected, too much upgrading mess.)
> >
> > 2. Provide mapping between index AM strategy numbers and btree strategy
> >   numbers.  (This doesn't have a space for non-btree operations like
> >   overlaps.)
>
> I agree that neither of these options are good, for the reasons you mention.
>
> > 3. Provide mapping between index AM strategy numbers and the existing
> >   RT*StrategyNumber numbers.  (We can expand the set as we want.)
> >   (This is what the existing mapping function for gist does.)
>
> The point of such a mapping is that core code outside any index AM can know what an AM is claiming it can do when it
advertisesthat it implements one of these strategy numbers.  We don't need any new entries in that mapping until some
corefeature depends on the semantics of the new entry.  Right now, all six of the btree comparators (including
not-equal)have semantics that are used outside AMs by functions like match_clause_to_ordering_op().  If any index AM
authorcomes along and creates an index AM which purports to provide these six semantics but actually does something
semanticallyinconsistent with what the backend thinks these mean, that index AM is totally at fault when, for example,
ORDERBY returns the wrong results. 
>
> On the other hand, if we add RTOverlapStrategyNumber to the common framework of strategy numbers, without anything
outsidean index AM depending on that, how is an index AM author to know exactly how an "overlaps" operator is supposed
tobehave?  No doubt, brin, gist, spgist, and friends all have their own understanding of what RTOverlapStrategyNumber
means,but how is a new index AM supposed to know if it has analogized that concept correctly to its own operator?  And
ifseveral major versions later, you come along to create some feature, let's say a logical replication feature
dependingon "overlaps" semantics, how are you to know whether all the index AMs in the wild which advertise they
providean "overlaps" operator will work correctly with your new feature?  When logical replication breaks, who is at
fault? Perversely, knowing that RTOverlapStrategyNumber is already in the list, you would likely implement the new
logicalreplication feature on some new strategy number, perhaps naming it RTLogicalOverlapStrategyNumber, to avoid such
conflicts.
>
> The RT*StrategyNumber list is much too long, containing many such problematic entries.  We should not, in my opinion,
addthese to the list prior to some new feature which depends on them, such as a planner or executor optimization. 
>
> > 4. Provide mapping between index AM strategy numbers and some
> >   completely new set of numbers/symbols.
>
> This is fine, if the new set is sufficiently restricted.  However, as mentioned below, the set of sufficiently
restrictedvalues is identical to what we currently define as a RowCompareType.  It creates needless code churn to throw
thataway and replace it with a new name. 
>
> > 5. Provide mapping between index AM strategy numbers and
> >   RowCompareType (with some small extensions).  This is what this
> >   patch does.
>
> As the patch author, obviously this is the one I chose.  The "small extensions" are just to handle "no such value"
typecases. 
>
>
> —
> Mark Dilger
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


Hi! Can we please have a rebased version of this patch series?

--
Best regards,
Kirill Reshke