| From: | Neil Chen <carpenter(dot)nail(dot)cz(at)gmail(dot)com> |
|---|---|
| To: | Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix GetOperatorFromCompareType |
| Date: | 2025-11-17 07:17:56 |
| Message-ID: | CAA3qoJnJhum8Vj3Bq8_ruRLLdH40myrLxrzpjRQ1bhLjTsJZYg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Paul,
On Mon, Nov 17, 2025 at 2:34 PM Paul A Jungwirth <
pj(at)illuminatedcomputing(dot)com> wrote:
> Hi Hackers,
>
> I found a few problems with GetOperatorFromCompareType and fixed them here.
>
> First of all, the comment was out of date: we never return
> InvalidStrategy; instead we ereport.
>
> You're right.
> Second, we were potentially using uninitialized Oids to build error
> messages (if get_opclass_opfamily_and_input_type failed). If that
> function fails, we should just die. In fact since get_opclass_method
> just succeeded, which makes the same lookup, how could it ever fail? I
> don't think we need to try very hard to build a fancy message.
>
> Failing right away simplifies the logic, because we only reach the
> bottom of the function one way. And I think we can make things ever
> clearer by inverting the conditional, so it acts like a guard, and we
> can avoid some nesting.
>
> +1.
After your patch changes, the line '*opid = InvalidOid;' seems removable.
Also, if the second validation check of opclass after 'get_opclass_method'
feels a bit odd, moving 'get_opclass_opfamily_and_input_type' to the very
top would work -- purely for visual clarity. :)
--
Ze Chen (Neil)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2025-11-17 07:28:45 | Re: Use exact nullingrels matches for NestLoopParams |
| Previous Message | Joel Jacobson | 2025-11-17 07:04:57 | Re: Optimize LISTEN/NOTIFY |