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

From Ashutosh Bapat
Subject Re: Enumize logical replication message actions
Date
Msg-id CAG-ACPWdoD=NUOjU0=SbhWti6quphzEoLqr44ME-aVyaURYCKg@mail.gmail.com
Whole thread Raw
In response to Re: Enumize logical replication message actions  (Andres Freund <andres@anarazel.de>)
Responses Re: Enumize logical replication message actions  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
List pgsql-hackers
Thanks Andres for your review. Thanks Li, Horiguchi-san and Amit for your comments.

On Tue, 20 Oct 2020 at 04:57, Andres Freund <andres@anarazel.de> wrote:
Hi,

On 2020-10-16 12:55:26 +0530, Ashutosh Bapat wrote:
> Here's a patch simplifying that for top level logical replication
> messages.

I think that's a good plan. One big benefit for me is that it's much
easier to search for an enum than for a single letter
constant. Including searching for all the places that deal with any sort
of logical rep message type. 


>  void
>  logicalrep_write_begin(StringInfo out, ReorderBufferTXN *txn)
>  {
> -     pq_sendbyte(out, 'B');          /* BEGIN */
> +     pq_sendbyte(out, LOGICAL_REP_MSG_BEGIN);                /* BEGIN */

I think if we have the LOGICAL_REP_MSG_BEGIN we don't need the /* BEGIN */.

Yes. Fixed all places.

I have attached two places - 0001 which is previous 0001 patch with your comments addressed.

0002 adds wrappers on top of pq_sendbyte() and pq_getmsgbyte() to send and receive a logical replication message type respectively. These wrappers add more protection to make sure that the enum definitions fit one byte. This also removes the default case from apply_dispatch() so that we can detect any LogicalRepMsgType not handled by that function.

These two patches are intended to be committed together as a single commit. For now the second one is separate so that it's easy to remove the changes if they are not acceptable.

--
Best Wishes,
Ashutosh
Attachment

pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: [Patch] Optimize dropping of relation buffers using dlist
Next
From: Thomas Munro
Date:
Subject: Re: [Patch] Optimize dropping of relation buffers using dlist