Re: Index AM API cleanup

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(at)eisentraut(dot)org
> <mailto:peter(at)eisentraut(dot)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