Re: Enumize logical replication message actions - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: Enumize logical replication message actions
Date
Msg-id CAG-ACPWH-0An8KgNSgwmCosjU4q=rKHLUMyQu81EJnERDd0MNw@mail.gmail.com
Whole thread Raw
In response to Re: Enumize logical replication message actions  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
List pgsql-hackers


On Fri, 23 Oct 2020 at 17:02, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
At Fri, 23 Oct 2020 19:53:00 +1100, Peter Smith <smithpb2250@gmail.com> wrote in
> On Fri, Oct 23, 2020 at 5:20 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in
> > > On 2020-Oct-22, Ashutosh Bapat wrote:
> > >
> > > > On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi <horikyota.ntt@gmail.com>
> > > > wrote:
> > >
> > > > > pg_send_logicalrep_msg_type() looks somewhat too-much.  If we need
> > > > > something like that we shouldn't do this refactoring, I think.
> > > >
> > > > Enum is an integer, and we want to send byte. The function asserts that the
> > > > enum fits a byte. If there's a way to declare byte long enums I would use
> > > > that. But I didn't find a way to do that.
>
> The pq_send_logicalrep_msg_type() function seemed a bit overkill to me.

Ah, yes, it is what I meant. I didn't come up with the word "overkill".

> The comment in the LogicalRepMsgType enum will sufficiently ensure
> nobody is going to accidentally add any bad replication message codes.
> And it's not like these are going to be changed often.

Agreed.

> Why not simply downcast your enums when calling pq_sendbyte?
> There are only a few of them.
>
> e.g. pq_sendbyte(out, (uint8)LOGICAL_REP_MSG_STREAM_COMMIT);

If you are worried about compiler warning, that explicit cast is not
required. Even if the symbol is larger than 0xff, the upper bytes are
silently truncated off.


I agree with Peter that the prologue of  LogicalRepMsgType is enough.

I also agree with Kyotaro, that explicit cast is unnecessary.

All this together makes the second patch useless. Removed it. Instead used Kyotaro's idea in previous mail.

PFA updated patch.

--
Best Wishes,
Ashutosh
Attachment

pgsql-hackers by date:

Previous
From: Dave Page
Date:
Subject: Re: git clone failed in windows
Next
From: Amit Kapila
Date:
Subject: Re: Enumize logical replication message actions