Rename functions to alloc/free things in reorderbuffer.c
I noticed some weird naming conventions in reorderbuffer.c which are
leftovers from a long time ago when reorderbuffer.c maintained its own
small memory pools to reduce palloc/pfree overhead. For example:
extern Oid *ReorderBufferGetRelids(ReorderBuffer *rb, int nrelids);
extern void ReorderBufferReturnRelids(ReorderBuffer *rb, Oid *relids);
ReorderBufferGetRelids allocates an array with MemoryContextAlloc, and
ReorderBufferReturnRelids just calls pfree. The pools are long gone, and
now the naming looks weird.
Attached patch renames those functions and other such functions to use
the terms Alloc/Free. I actually wonder if we should go further and
remove these functions altogether, and change the callers to call
MemoryContextAlloc directly. But I didn't do that yet.
Any objections?
--
Heikki Linnakangas
Neon (https://neon.tech)
Attachments:
0001-Rename-alloc-free-functions-in-reorderbuffer.c.patchtext/x-patch; charset=UTF-8; name=0001-Rename-alloc-free-functions-in-reorderbuffer.c.patchDownload+64-67
Heikki Linnakangas <hlinnaka@iki.fi> writes:
ReorderBufferGetRelids allocates an array with MemoryContextAlloc, and
ReorderBufferReturnRelids just calls pfree. The pools are long gone, and
now the naming looks weird.
Attached patch renames those functions and other such functions to use
the terms Alloc/Free. I actually wonder if we should go further and
remove these functions altogether, and change the callers to call
MemoryContextAlloc directly. But I didn't do that yet.
Yeah, that is very confusing, especially since nearby code uses
names like ReorderBufferGetFoo for functions that are lookups not
allocations. +1 for Alloc/Free where that's an accurate description.
Given that a lot of these are not just one-liners, I'm not sure that
getting rid of the ones that are would help much.
regards, tom lane
On 12/03/2025 21:31, Tom Lane wrote:
Heikki Linnakangas <hlinnaka@iki.fi> writes:
ReorderBufferGetRelids allocates an array with MemoryContextAlloc, and
ReorderBufferReturnRelids just calls pfree. The pools are long gone, and
now the naming looks weird.Attached patch renames those functions and other such functions to use
the terms Alloc/Free. I actually wonder if we should go further and
remove these functions altogether, and change the callers to call
MemoryContextAlloc directly. But I didn't do that yet.Yeah, that is very confusing, especially since nearby code uses
names like ReorderBufferGetFoo for functions that are lookups not
allocations. +1 for Alloc/Free where that's an accurate description.
Committed, thanks!
Given that a lot of these are not just one-liners, I'm not sure that
getting rid of the ones that are would help much.
Fair
--
Heikki Linnakangas
Neon (https://neon.tech)