Re: Fix GetOperatorFromCompareType

On Sun, Jan 4, 2026 at 6:46 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Fri, 21 Nov 2025 at 05:45, Paul A Jungwirth
> <pj(at)illuminatedcomputing(dot)com> wrote:
> > Thanks for the review! Here is a patch with your suggestions incorporated.
>
> I had a look at this. I agree the code could be made simpler, but I
> don't see any window for "potentially using uninitialized Oids to
> build error messages". I think you must be talking about the final
> ERROR message using opfamily and opcintype, but it seems to me like
> the call to get_opclass_method() would ERROR if the opclass couldn't
> be found and there's no window for the opclass to be removed before
> the call to get_opclass_opfamily_and_input_type() as we don't process
> catcache invalidations in between.
>
> That makes me think there's no live issue here, so it's more just
> about a cleanup and simplification.

Thanks for taking a look! I agree it should never happen, although it
seems easy for future code to introduce a problem, so I think cleaning
it up is also preventative.

> I split your patch into two and wrote a comment to explain about
> ERRORs are raised on failed lookups. We should likely fix that in v18
> since the comment is misleading, but for 0002, since nothing seems
> broken, then it seems safer just to do that one in master.
>
> What do you think?

This split makes sense to me and seems fine.

If you want to be very strict, I think this should be a semicolon, not a comma:

The comment claimed *strat got set to InvalidStrategy when the function
lookup fails. This isn't true, an ERROR is raised when that happens.

Yours,

--
Paul ~{:-)
pj(at)illuminatedcomputing(dot)com