Re: nbtree: Cache operator family OID in _bt_setup_array_cmp

Поиск
Список
Период
Сортировка
От Chao Li
Тема Re: nbtree: Cache operator family OID in _bt_setup_array_cmp
Дата
Msg-id CAEoWx2mW51fL7meN=a7F_kyMD=_tXC=Gf6OhDT9Sex2gN618vQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: nbtree: Cache operator family OID in _bt_setup_array_cmp  (Peter Eisentraut <peter@eisentraut.org>)
Список pgsql-hackers


On Jan 7, 2026, at 22:45, Peter Eisentraut <peter@eisentraut.org> wrote:

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 performs multiple 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 functions within the same file (such as _bt_skiparray_strat_decrement, _bt_preprocess_array_keys, etc.), making the code more consistent and readable.

The assignment is placed after the early-return check for (elemtype == opcintype) to ensure we only perform the array indexing 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.

Fair point -- thanks for the clarification. You’re right, I didn’t mean to imply any measurable performance benefit here.

The motivation is readability and local consistency rather than caching in an optimization sense. In nbtpreprocesskeys.c, several nearby helper functions already assign rel->rd_opfamily[skey->sk_attno - 1] to a local variable and then use that for opfamily lookups. This change brings _bt_setup_array_cmp() in line with that existing pattern and avoids repeating the same expression, which I consider a small improvement to readability and maintainability.

I have updated the commit message to remove the efficiency language and focus on clarity and consistency instead. Please see attached v2.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/




Вложения

В списке pgsql-hackers по дате отправления: