Re: Enumize logical replication message actions

On Fri, 16 Oct 2020 at 14:06, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
wrote:

> 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:
>
> >
> > 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.
>

I think we converting those into macros/enums will help but for now I have
tackled only the top level message types.

>
> 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.
>

That's exactly the problem I am trying to solve. Take for example 'B' as I
have mentioned before. That string literal appears in 64 different places
in the master branch. Which of those are the ones related to a "BEGIN"
message in logical replication protocol is not clear, unless I thumb
through each of those. In PostgresMain it's used to indicate a BIND
message. Which of those 64 instances are also using 'B' to mean a bind
message? Using enums or macros makes it clear. Just look
up LOGICAL_REP_MSG_BEGIN. Converting all 'B' to their respective macros
will help but might be problematic for back-patching. So that's arguable.
But doing that in something as new as logical replication will be helpful,
before it gets too late to change that.

Further logical repliation protocol is using the same literal e.g. 'O' to
mean origin in some places and old tuple in some other. While comments
there help, it's not easy to locate all the code that deals with one
meaning or the other. This change will help with that. Another reason as to
why logical replication.
--
Best Wishes,
Ashutosh