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 <pe...@eisentraut.org
<mailto:pe...@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
