Date: Wed, 3 Dec 2025 11:17:43 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Jelte Fennema-Nio <postgres@jeltef.nl>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Safer hash table initialization macro
Message-ID: <aTAcV/OF0uAfT4rL@ip-10-97-1-34.eu-west-3.compute.internal>
References: <aS2b3LoUypW1/Gdz@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAGECzQSHb6FxuSdYNY7tt8SKFw2dianACvkExrWRmTBfy-CkjQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="Mm7e/lTiPFa8Qm1V"
Content-Disposition: inline
In-Reply-To: 
 <CAGECzQSHb6FxuSdYNY7tt8SKFw2dianACvkExrWRmTBfy-CkjQ@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aTAcV/OF0uAfT4rL%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk


--Mm7e/lTiPFa8Qm1V
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

On Mon, Dec 01, 2025 at 03:44:41PM +0100, Jelte Fennema-Nio wrote:
> On Mon, 1 Dec 2025 at 14:45, Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> > Thoughts?
> 
> I think the hashtable creation API in postgres is so terrible that it
> actively discourages usage.

Thanks for sharing your thoughts!

> So I'm definitely in favor of improving this API (probably by adding a
> few new functions). I have a few main thoughts on what could be
> improved:
> 
> 1. Automatically determine keysize and entrysize given a keymember and
> entrytype (like you suggested).

PFA a patch introducing and using the new macro. Note that it also introduces
HASH_ELEM_INIT_FULL for the rare cases where the whole struct is the
key.

Also one case remains untouched:

$ git grep "entrysize = sizeof" "*.c"
src/backend/replication/logical/relation.c:     ctl.entrysize = sizeof(LogicalRepRelMapEntry);

That's because the key is a member of a nested struct so that the new
macro can not be used. As there is only one occurrence of it, I think we can keep
it as it is. But we could create a dedicated macro for those cases if we feel
the need. Now that I'm writing this, that might be a better idea: that way we'd
avoid any "entrysize/keysize = " in the .c files.

Also a nice side effect of using the macros:

138 insertions(+), 203 deletions(-)

> 2. Autodect most of the flags.
>     a. HASH_PARTITION, HASH_SEGMENT, HASH_FUNCTION, HASH_DIRSIZE,
> HASH_COMPARE, HASH_KEYCOPY, HASH_ALLOC can all be simply be detected
> based on the relevant fields from HASHCTL. Passing them in explicitly
> is just duplication that causes code noise and is easy to forget
> accidentally.
>     b. HASH_ELEM is useless noise because it is required
>     c. HASH_BLOBS could be the default (and maybe use HASH_STRINGS by
> default if the keytype is char*)
> 3. Default to CurrentMemoryContext instead of TopMemoryContext. Like
> all of the other functions that allocate, because right now it's way
> too easy to accidentally use TopMemoryContext when you did not intend
> to.
> 4. Have a creation function that doesn't require HASHCTL at all (just
> takes entrytype and keymember and maybe a version of this that takes a
> memorycontext).

Thanks for the above suggestions! I did not think so deep as you did during
your Citus time, but will think about those too. I suggest we move forward one
step at a time, first step being the new macros. Does that make sense to you?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

--Mm7e/lTiPFa8Qm1V
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0001-Safer-hash-table-initialization-macro.patch"