Re: Index AM API cleanup

On Thu, 31 Oct 2024 at 15:02, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>
>
>
> > On Oct 30, 2024, at 12:54 PM, Peter Eisentraut <peter(at)eisentraut(dot)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 advertises that it implements one of these strategy numbers. We don't need any new entries in that mapping until some core feature 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 author comes along and creates an index AM which purports to provide these six semantics but actually does something semantically inconsistent with what the backend thinks these mean, that index AM is totally at fault when, for example, ORDER BY returns the wrong results.
>
> On the other hand, if we add RTOverlapStrategyNumber to the common framework of strategy numbers, without anything outside an index AM depending on that, how is an index AM author to know exactly how an "overlaps" operator is supposed to behave? 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 if several major versions later, you come along to create some feature, let's say a logical replication feature depending on "overlaps" semantics, how are you to know whether all the index AMs in the wild which advertise they provide an "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 logical replication 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, add these 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 restricted values is identical to what we currently define as a RowCompareType. It creates needless code churn to throw that away 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" type cases.
>
>
> —
> 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