Обсуждение: Fix GetOperatorFromCompareType

Поиск
Список
Период
Сортировка

Fix GetOperatorFromCompareType

От
Paul A Jungwirth
Дата:
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.

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.

Based on acbc9beaae.

Yours,

-- 
Paul              ~{:-)
pj@illuminatedcomputing.com

Вложения

Re: Fix GetOperatorFromCompareType

От
Neil Chen
Дата:
Hi Paul, 

On Mon, Nov 17, 2025 at 2:34 PM Paul A Jungwirth <pj@illuminatedcomputing.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/