Re: [dynahash] do not refill the hashkey after hash_search - Mailing list pgsql-hackers

From John Naylor
Subject Re: [dynahash] do not refill the hashkey after hash_search
Date
Msg-id CAFBsxsGXWZBfPFF_6F6td6Oq5ABJDq-ACAHa7bshDcEEKOD5VQ@mail.gmail.com
Whole thread Raw
In response to [dynahash] do not refill the hashkey after hash_search  (Junwang Zhao <zhjwpku@gmail.com>)
Responses Re: [dynahash] do not refill the hashkey after hash_search
List pgsql-hackers

On Wed, Sep 13, 2023 at 1:47 PM Junwang Zhao <zhjwpku@gmail.com> wrote:
>
> Hi hackers,
>
> It's not necessary to fill the key field for most cases, since
> hash_search has already done that for you. For developer that
> using memset to zero the entry structure after enter it, fill the
> key field is a must, but IMHO that is not good coding style, we
> really should not touch the key field after insert it into the
> dynahash.

- memset(part_entry, 0, sizeof(LogicalRepPartMapEntry));
- part_entry->partoid = partOid;
+ Assert(part_entry->partoid == partOid);
+ memset(entry, 0, sizeof(LogicalRepRelMapEntry));

This is making an assumption that the non-key part of LogicalRepPartMapEntry will never get new members. Without knowing much about this code, it seems like a risk in the abstract.

> This patch fixed some most abnormal ones, instead of refilling the
> key field of primitive types, adding some assert might be a better
> choice.

Taking a quick look, I didn't happen to see any existing asserts of this sort, so the patch doesn't seem to be making things more "normal". I did see a few instances of /* hash_search already filled in the key */, so if we do anything at all here, we might prefer that.

- hash_search(uncommitted_enums, serialized++, HASH_ENTER, NULL);
+ (void) hash_search(uncommitted_enums, serialized++, HASH_ENTER, NULL);

I prefer explicit (void) for new code, but others may disagree. I don't think we have a preferred style for this, so changing current usage will just cause unnecessary code churn.

--
John Naylor
EDB: http://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Hongxu Ma
Date:
Subject: Re: PSQL error: total cell count of XXX exceeded
Next
From: Junwang Zhao
Date:
Subject: Re: [dynahash] do not refill the hashkey after hash_search