Re: Enumize logical replication message actions

At Fri, 16 Oct 2020 08:08:40 +0000, Li Japin <japinli(at)hotmail(dot)com> wrote in
>
> > On Oct 16, 2020, at 3:25 PM, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >
> > Hi All,
> > Logical replication protocol uses single byte character to identify
> > different chunks of logical repliation messages. The code uses
> > character literals for the same. These literals are used as bare
> > constants in code as well. That's true for almost all the code that
> > deals with wire protocol. With that it becomes difficult to identify
> > the code which deals with a particular message. For example code that
> > deals with message type 'B'. In various protocol 'B' has different
> > meaning and it gets difficult and time consuming to differentiate one
> > usage from other and find all places which deal with one usage. Here's
> > a patch simplifying that for top level logical replication messages.
> >
> > I think I have covered the places that need change. But I might have
> > missed something, given that these literals are used at several other
> > places (a problem this patch tries to fix :)).
> >
> > Initially I had used #define for the same, but Peter E suggested using
> > Enums so that switch cases can detect any remaining items along with
> > stronger type checks.
> >
> > Pavan offleast suggested to create a wrapper
> > pg_send_logical_rep_message() on top of pg_sendbyte(), similarly for
> > pg_getmsgbyte(). I wanted to see if this change is acceptable. If so,
> > I will change that as well. Comments/suggestions welcome.
> >
> > --
> > Best Wishes,
> > Ashutosh Bapat
> > <0001-Enumize-top-level-logical-replication-actions.patch>
>
> What about ’N’ for new tuples, ‘O’ for old tuple follows, ‘K’ for old key follows?
> Those are also logical replication protocol message, I think.

They are flags stored in a message so they can be seen as different
from the message type letters.

Anyway if the values are determined after some meaning, I'm not sure
enumerize them is good thing or not. In other words, 'U' conveys
almost same amount of information with LOGICAL_REP_MSG_UPDATE in the
context of logical replcation protocol.

We have the same code pattern in PostgresMain and perhaps we don't
going to change them into enums.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center