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

On Wed, May 12, 2021 at 1:02 AM vignesh C <vignesh21@gmail.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)"

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

~~~

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)"

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

~~~

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

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