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

On Wed, May 12, 2021 at 3:36 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Wed, May 12, 2021 at 1:02 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > Thanks for the comments, Attached v3 patch has the changes as suggested.
>
> This v3 mostly looks good to me now except for some minor comments
> about the flags.
>
> ~~~
>
> 1. Commit flags
>
> @@ -6534,11 +6536,11 @@ Commit
> </varlistentry>
> <varlistentry>
> <term>
> - Int8
> + Uint8(0)
> </term>
> <listitem>
> <para>
> - Flags; currently unused (must be 0).
> + Flags (uint8(0)); currently unused (must be 0).
> </para>
> </listitem>
> </varlistentry>
>
> a) There is no data type Uint8. That should be "Int8(0)"
>

Modified.

> b) I think the "(0)" does not belong in the description part.
> e.g. change to "Flags (uint8); currently unused (must be 0)."
>

Modified

> ~~~
>
> 2. Stream Commit flags
>
> @@ -7276,7 +7284,7 @@ Stream Commit
> </term>
> <listitem>
> <para>
> - Flags; currently unused (must be 0).
> + Flags (uint8(0)); currently unused (must be 0).
> </para>
> </listitem>
> </varlistentry>
>
> a) The data type should say "Int8(0)"
>

Modified.

> b) I think the "(0)" does not belong in the description part.
> e.g. change to "Flags (uint8); currently unused (must be 0)."
>

Modified.

> ~~~
>
> 3. I felt that saying "(must be 0)" for those unused flag descriptions
> is unnecessary since that is exactly what "Int8(0)" already means.
> e.g. consider change to just say "Flags (uint8); currently unused."

Modified.

Thanks for the comments. Attached v4 patch has the fix for the same.

Regards,
Vignesh