Re: [dynahash] do not refill the hashkey after hash_search

2023-11-30 Thread John Naylor
On Thu, Sep 14, 2023 at 3:28 PM Junwang Zhao wrote: > > Add a v2 with some change to fix warnings about unused-parameter. > > I will add this to Commit Fest. Pushed v2 after removing asserts, as well as the unnecessary cast that I complained about earlier. Some advice: I was added as a reviewer

Re: [dynahash] do not refill the hashkey after hash_search

2023-10-25 Thread Nathan Bossart
On Wed, Oct 25, 2023 at 12:48:52PM +0700, John Naylor wrote: > On Wed, Oct 25, 2023 at 12:21 PM Tom Lane wrote: >> >> John Naylor writes: >> > I'd prefer just adding "Assert(hentry->event == oldn);" and declaring >> > hentry PG_USED_FOR_ASSERTS_ONLY. >> >> I'm not aware of any other places where

Re: [dynahash] do not refill the hashkey after hash_search

2023-10-24 Thread John Naylor
On Wed, Oct 25, 2023 at 12:21 PM Tom Lane wrote: > > John Naylor writes: > > I'd prefer just adding "Assert(hentry->event == oldn);" and declaring > > hentry PG_USED_FOR_ASSERTS_ONLY. > > I'm not aware of any other places where we have Asserts checking > that hash_search() honored its contract.

Re: [dynahash] do not refill the hashkey after hash_search

2023-10-24 Thread Tom Lane
John Naylor writes: > I'd prefer just adding "Assert(hentry->event == oldn);" and declaring > hentry PG_USED_FOR_ASSERTS_ONLY. I'm not aware of any other places where we have Asserts checking that hash_search() honored its contract. Why do we need one here? regards, tom

Re: [dynahash] do not refill the hashkey after hash_search

2023-10-24 Thread John Naylor
On Fri, Oct 13, 2023 at 10:07 AM Nathan Bossart wrote: > > On Thu, Sep 14, 2023 at 04:28:26PM +0800, Junwang Zhao wrote: > > Add a v2 with some change to fix warnings about unused-parameter. > > > > I will add this to Commit Fest. > > This looks reasonable to me. I've marked the commitfest entry

Re: [dynahash] do not refill the hashkey after hash_search

2023-10-12 Thread Nathan Bossart
On Thu, Sep 14, 2023 at 04:28:26PM +0800, Junwang Zhao wrote: > Add a v2 with some change to fix warnings about unused-parameter. > > I will add this to Commit Fest. This looks reasonable to me. I've marked the commitfest entry as ready-for-committer. I will plan on committing it in a couple

Re: [dynahash] do not refill the hashkey after hash_search

2023-09-14 Thread Junwang Zhao
On Wed, Sep 13, 2023 at 5:28 PM John Naylor wrote: > > > On Wed, Sep 13, 2023 at 3:46 PM Junwang Zhao wrote: > > > > On Wed, Sep 13, 2023 at 4:22 PM John Naylor > > wrote: > > > > - memset(part_entry, 0, sizeof(LogicalRepPartMapEntry)); > > > - part_entry->partoid = partOid; > > > +

Re: [dynahash] do not refill the hashkey after hash_search

2023-09-13 Thread John Naylor
On Wed, Sep 13, 2023 at 3:46 PM Junwang Zhao wrote: > > On Wed, Sep 13, 2023 at 4:22 PM John Naylor > wrote: > > - memset(part_entry, 0, sizeof(LogicalRepPartMapEntry)); > > - part_entry->partoid = partOid; > > + Assert(part_entry->partoid == partOid); > > + memset(entry, 0,

Re: [dynahash] do not refill the hashkey after hash_search

2023-09-13 Thread Junwang Zhao
On Wed, Sep 13, 2023 at 4:22 PM John Naylor wrote: > > > On Wed, Sep 13, 2023 at 1:47 PM Junwang Zhao 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

Re: [dynahash] do not refill the hashkey after hash_search

2023-09-13 Thread John Naylor
On Wed, Sep 13, 2023 at 1:47 PM Junwang Zhao 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

[dynahash] do not refill the hashkey after hash_search

2023-09-13 Thread Junwang Zhao
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