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 CAFBsxsEr_kz7_Z72ndJ-wEAU2U0UimhAXkXpg9fgC+Y4-A8y9w@mail.gmail.com
Whole thread Raw
In response to Re: [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 3:46 PM Junwang Zhao <zhjwpku@gmail.com> wrote:
>
> On Wed, Sep 13, 2023 at 4:22 PM John Naylor
> <john.naylor@enterprisedb.com> wrote:

> > - 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.
>
> What do you mean by 'the non-key part of LogicalRepPartMapEntry will
> never get new members'?

I mean, if this struct:

> typedef struct LogicalRepPartMapEntry
> {
>         Oid partoid; /* LogicalRepPartMap's key */
>         LogicalRepRelMapEntry relmapentry;
> } LogicalRepPartMapEntry;

...gets a new member, it will not get memset when memsetting "relmapentry".

> > 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.
>
> There are some code using assert for this sort, for example in
> *ReorderBufferToastAppendChunk*:

> and in *rebuild_database_list*, tom commented that the key has already
> been filled, which I think
> he was trying to tell people no need to assign the key again.

Okay, we have examples of each.

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

pgsql-hackers by date:

Previous
From: Maxim Orlov
Date:
Subject: Re: ALTER COLUMN ... SET EXPRESSION to alter stored generated column's expression
Next
From: Amit Kapila
Date:
Subject: Re: [PoC] pg_upgrade: allow to upgrade publisher node