Horton
Menu

Re: Index AM API cleanup

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-03-18T11:11:05Z
Lists: pgsql-hackers 
I have committed these four patches (squashed into three).  I made the 
error handling change in 0003 that you requested, and also the error 
handling change in 0002 discussed in an adjacent message.


On 12.03.25 16:52, Mark Dilger wrote:
> 
> 
> On Wed, Mar 12, 2025 at 7:00 AM Peter Eisentraut <peter@eisentraut.org 
> <mailto:peter@eisentraut.org>> wrote:
> 
>     And another small patch set extracted from the bigger one, to keep
>     things moving along:
> 
>     0001: Add get_opfamily_method() in lsyscache.c, from your patch set.
> 
> 
> Right, this came from v21-0006-*, with a slight code comment change and 
> one variable renaming.  It is ready for commit.
> 
>     0002: Add get_opfamily_member_for_cmptype().  This was called
>     get_opmethod_member() in your patch set, but I think that name wasn't
>     quite right.  I also removed the opmethod argument, which was rarely
>     used and is somewhat redundant.
> 
> 
> This is also taken from v21-0006-*.  The reason I had an opmethod 
> argument was that some of the callers of this function already know the 
> opmethod, and without the argument, this function has to look up the 
> opmethod from the syscache again.  Whether that makes a measurable 
> performance difference is an open question.
> Your version is ready for commit.  If we want to reintroduce the 
> opmethod argument for performance reasons, we can always circle back to 
> that in a later commit.
> 
>     0003 and 0004 are enabling non-btree unique indexes for partition keys
> 
> 
> You refactored v21-0011-* into v21.2-0003-*, in which an error gets 
> raised about a missing operator in a slightly different part of the 
> logic.  I am concerned that the new positioning of the check-and-error 
> might allow the flow of execution to reach the Assert(idx_eqop) 
> statement in situations where the user has defined an incomplete 
> opfamily or opclass.  Such a condition should raise an error about the 
> missing operator rather than asserting.
> 
> In particular, looking at the control logic:
> 
>                         if (stmt->unique && !stmt->iswithoutoverlaps)
>                          {
>                                 ....
>                          }
>                          else if (exclusion)
>                                 ....;
> 
>                          Assert(idx_eqop);
> 
> I cannot prove to myself that the assertion cannot trigger, because the 
> upstream logic before we reach this point *might* be filtering out all 
> cases where this could be a problem, but it is too complicated to 
> prove.  Even if it is impossible now, this is a pretty brittle piece of 
> code after applying the patch.
> 
> Any chance you'd like to keep the patch closer to how I had it in 
> v21-0011-* ?
> 
>     and materialized views.  These were v21-0011 and v21-0012, except that
>     I'm combining the switch from strategies to compare types (which was in
>     v21-0006 or so) with the removal of the btree requirements.
> 
> 
> v21.2-0004-* is ready for commit.
> 
> —
> Mark Dilger
> EnterpriseDB:http://www.enterprisedb.com <http://www.enterprisedb.com/>
> The Enterprise PostgreSQL Company

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