Horton
Menu

Re: Index AM API cleanup

Kirill Reshke <reshkekirill@gmail.com>

View thread
From: Kirill Reshke <reshkekirill@gmail.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Andrew Dunstan <andrew.dunstan@enterprisedb.com>, Alex Wang <alex.wang@enterprisedb.com>
Date: 2024-11-16T17:10:00Z
Lists: 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 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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Relax ordering-related hardcoded btree requirements in planning

  2. Support non-btree indexes in get_actual_variable_range()

  3. Convert PathKey to use CompareType

  4. Generalize index support in network support function

  5. Update a code comment

  6. Allow non-btree unique indexes for matviews

  7. Allow non-btree unique indexes for partition keys

  8. Add some opfamily support functions to lsyscache.c

  9. Simplify and generalize PrepareSortSupportFromIndexRel()

  10. Generalize hash and ordering support in amapi

  11. Drop opcintype from index AM strategy translation API

  12. Allow non-btree speculative insertion indexes

  13. Support non-btree indexes for foreign keys

  14. Integrate GistTranslateCompareType() into IndexAmTranslateCompareType()

  15. Convert strategies to and from compare types

  16. Move CompareType to separate header file

  17. Add get_opfamily_name() function

  18. Rename GistTranslateStratnum() to GistTranslateCompareType()

  19. Change gist stratnum function to use CompareType

  20. Rename RowCompareType to CompareType

  21. Improve slightly misleading internal error message

  22. Track scan reversals in MergeJoin

  23. Track sort direction in SortGroupClause

  24. Add stratnum GiST support function

  25. Add amgettreeheight index AM API routine