Re: Corrected documentation of data type for the logical replication message formats.

On Sun, Aug 1, 2021 at 4:11 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Sat, Jul 31, 2021 at 7:00 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > vignesh C <vignesh21(at)gmail(dot)com> writes:
> > [ v6-0001-Included-the-actual-datatype-used-in-logical-repl.patch ]
> >
> > I see what you want to do here, but the way you did it seems quite
> > detrimental to the readability of the field descriptions.
> > Parenthesized interjections should be used sparingly.
> >
> > I'm inclined to think that the equivalent data type is part of the
> > field data type specification, and thus that we ought to put it in
> > the data type part of each entry. So we'd have something like
> >
> > <varlistentry>
> > <term>
> > Int64 (XLogRecPtr)
> > </term>
> > <listitem>
> > <para>
> > The final LSN of the transaction.
> > </para>
> > </listitem>
> > </varlistentry>
> >
> > instead of what you did here. Parentheses might not be the best
> > punctuation to use, given the existing convention about parenthesized
> > specific values, but we could probably settle on some other markup.
> > Or just ignore the ambiguity.
>
> +1 to change it like suggested above.
>
> The specific value for the flags might then look like below, but that
> does not look too bad to me.
>
> <term>
> Int8 (uint8) (0)
> </term>

I felt we can change it like:
<term>
Int8(0) (uint8)
</term>

I felt the flag value can be kept first followed by the data type since it
is used similarly for the other message types like below:
<term>
Byte1('C')
</term>

I have made changes in similar lines and posted the patch at [1].
Thoughts?

[1] -
https://www.postgresql.org/message-id/CALDaNm3sK75Mo%2BVzLmNGe29gYtJoeKHshAK0GDiAzfAj6LQPdw%40mail.gmail.com

Regards,
Vignesh