Re: nbtree: Cache operator family OID in _bt_setup_array_cmp

On 31.12.25 11:13, Chao Li wrote:
> 
> 
>> On Dec 31, 2025, at 18:04, Kirill Reshke <reshkekirill@gmail.com> wrote:
>>
>> On Wed, 31 Dec 2025 at 13:16, Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>> Hi Hackers,
>>>
>>> While reviewing a separate patch related to nbtree, I noticed that _bt_setup_array_cmp in nbtpreprocesskeys.c
performsmultiple redundant lookups of the operator family OID via rel->rd_opfamily[skey->sk_attno - 1].
 
>>>
>>> The attached patch caches this value in a local opfamily variable. This matches the pattern used in several other
functionswithin the same file (such as _bt_skiparray_strat_decrement, _bt_preprocess_array_keys, etc.), making the code
moreconsistent and readable.
 
>>>
>>> The assignment is placed after the early-return check for (elemtype == opcintype) to ensure we only perform the
arrayindexing when the cross-type comparison logic is actually reached.
 
>>>
>>> Best regards,
>>> --
>>> Chao Li (Evan)
>>> HighGo Software Co., Ltd.
>>> https://www.highgo.com/
>>>
>>
>>
>> Hi!
>> We do not cache anything here, this is just a refactoring for nbtree internals?
>>
> 
> Yes, just cache  rel->rd_opfamily[skey->sk_attno - 1] into a local variable.

But that's not caching, at least in a compiled language with an 
optimizing compiler.

Are you claiming that this has a performance impact, or that it makes 
the code easier to understand, or something else?  The patch isn't 
necessarily wrong, but a clear description of the motivation would be good.