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

On Mon, Aug 2, 2021 at 1:32 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> 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?

I agree. The specified value looks better when it comes first, as you did it.

~~

Option #1:
Int8(0) (uint8)
Int64 (XLogRecPtr)

Option #2:
Int8(0) [uint8]
Int64 [XLogRecPtr]

Option #3:
Int8(0) -- uint
Int64 -- XLogRecPtr

etc...

Probably my slight favourite is Option #2 above, but YMMV. Any format
you choose which is similar to those above is fine by me.

------
Kind Regards,
Peter Smith.
Fujitsu Australia