Re: Corrected documentation of data type for the logical replication message formats. - Mailing list pgsql-hackers

From Peter Smith
Subject Re: Corrected documentation of data type for the logical replication message formats.
Date
Msg-id CAHut+Ps3+qh-FfhHhzbv_aCmF45vWwT6PvtpTTTv7K0m058DwA@mail.gmail.com
Whole thread Raw
In response to Re: Corrected documentation of data type for the logical replication message formats.  (vignesh C <vignesh21@gmail.com>)
Responses Re: Corrected documentation of data type for the logical replication message formats.  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Mon, Aug 2, 2021 at 1:32 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Sun, Aug 1, 2021 at 4:11 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > On Sat, Jul 31, 2021 at 7:00 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > vignesh C <vignesh21@gmail.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
typeslike 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



pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Record a Bitmapset of non-pruned partitions
Next
From: Tom Lane
Date:
Subject: Re: [PATCH] proposal for regexp_count, regexp_instr, regexp_substr and regexp_replace