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

2021-08-02 Thread vignesh C
On Mon, Aug 2, 2021 at 9:10 PM Tom Lane wrote: > > Peter Smith writes: > > I agree. The specified value looks better when it comes first, as you did > > it. > > Actually, it looks to me like we don't have to resolve the question of > which should come first, because I don't see any cases where

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

2021-08-02 Thread Tom Lane
Peter Smith writes: > I agree. The specified value looks better when it comes first, as you did it. Actually, it looks to me like we don't have to resolve the question of which should come first, because I don't see any cases where it's useful to have both. I don't agree with appending "uint8"

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

2021-08-01 Thread Peter Smith
On Mon, Aug 2, 2021 at 1:32 AM vignesh C wrote: > > On Sun, Aug 1, 2021 at 4:11 PM Peter Smith wrote: > > > > On Sat, Jul 31, 2021 at 7:00 AM Tom Lane wrote: > > > > > > vignesh C writes: > > > [ v6-0001-Included-the-actual-datatype-used-in-logical-repl.patch ] > > > > > > I see what you want

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

2021-08-01 Thread vignesh C
On Sun, Aug 1, 2021 at 4:11 PM Peter Smith wrote: > > On Sat, Jul 31, 2021 at 7:00 AM Tom Lane wrote: > > > > vignesh C 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

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

2021-08-01 Thread vignesh C
On Sat, Jul 31, 2021 at 2:30 AM Tom Lane wrote: > > vignesh C 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

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

2021-08-01 Thread Peter Smith
On Sat, Jul 31, 2021 at 7:00 AM Tom Lane wrote: > > vignesh C 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

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

2021-07-30 Thread Tom Lane
vignesh C 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

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

2021-07-23 Thread vignesh C
On Fri, Jul 23, 2021 at 3:23 AM Peter Smith wrote: > > I think the patch maybe is not quite correct for all the flags. > > For example, > > @@ -7607,44 +7615,44 @@ are available since protocol version 3. > > Int8 > > -Flags; currently unused (must be 0). > +

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

2021-07-22 Thread Peter Smith
I think the patch maybe is not quite correct for all the flags. For example, @@ -7607,44 +7615,44 @@ are available since protocol version 3. Int8 -Flags; currently unused (must be 0). +Flags (uint8); currently unused. AFAIK, even though the flags are

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

2021-07-16 Thread vignesh C
On Fri, Jul 16, 2021 at 8:52 AM Peter Smith wrote: > > Hi Vignesh. > > FYI - Because the other patch [1] was pushed ahead of this one, I > think your patch now needs to be updated for the new message types > that were introduced. Thanks, I have made the changes for the same in the v5 patch

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

2021-07-15 Thread Peter Smith
Hi Vignesh. FYI - Because the other patch [1] was pushed ahead of this one, I think your patch now needs to be updated for the new message types that were introduced. -- [1]

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

2021-05-17 Thread vignesh C
On Thu, May 13, 2021 at 5:57 AM Peter Smith wrote: > > On Wed, May 12, 2021 at 11:09 PM vignesh C wrote: > ... > > > > Thanks for the comments. Attached v4 patch has the fix for the same. > > > > I have not tried this patch so I cannot confirm whether it applies or > renders OK, but just going

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

2021-05-12 Thread Peter Smith
On Wed, May 12, 2021 at 11:09 PM vignesh C wrote: ... > > Thanks for the comments. Attached v4 patch has the fix for the same. > I have not tried this patch so I cannot confirm whether it applies or renders OK, but just going by the v4 content this now LGTM. Kind Regards, Peter Smith.

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

2021-05-12 Thread vignesh C
On Wed, May 12, 2021 at 3:36 AM Peter Smith wrote: > > On Wed, May 12, 2021 at 1:02 AM vignesh C 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

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

2021-05-11 Thread Peter Smith
On Wed, May 12, 2021 at 1:02 AM vignesh C 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 -Int8 +

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

2021-05-11 Thread vignesh C
On Tue, May 11, 2021 at 9:09 AM Euler Taveira wrote: > > On Mon, May 10, 2021, at 10:45 AM, vignesh C wrote: > > I agree to specifying the actual dataypes like XLogRecPtr for lsn, > TimestampTz for timestamp, TransactionId for xid and Oid for the > object id. Attached v2 patch which is changed on

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

2021-05-11 Thread vignesh C
On Tue, May 11, 2021 at 8:06 AM Peter Smith wrote: > > On Mon, May 10, 2021 at 11:46 PM vignesh C wrote: > > > > On Sun, May 9, 2021 at 6:54 PM Euler Taveira wrote: > > > > > > On Sun, May 9, 2021, at 9:37 AM, vignesh C wrote: > > > > > > For some of the logical replication messages the data

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

2021-05-10 Thread Euler Taveira
On Mon, May 10, 2021, at 10:45 AM, vignesh C wrote: > I agree to specifying the actual dataypes like XLogRecPtr for lsn, > TimestampTz for timestamp, TransactionId for xid and Oid for the > object id. Attached v2 patch which is changed on similar lines. > Thoughts? Perhaps I didn't make myself

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

2021-05-10 Thread Peter Smith
On Mon, May 10, 2021 at 11:46 PM vignesh C wrote: > > On Sun, May 9, 2021 at 6:54 PM Euler Taveira wrote: > > > > On Sun, May 9, 2021, at 9:37 AM, vignesh C wrote: > > > > For some of the logical replication messages the data type documented > > was not correct, especially for lsn and xid. For

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

2021-05-10 Thread vignesh C
On Sun, May 9, 2021 at 6:44 PM Peter Smith wrote: > > On Sun, May 9, 2021 at 10:38 PM vignesh C wrote: > > > > Hi, > > > > For some of the logical replication messages the data type documented > > was not correct, especially for lsn and xid. For lsn actual datatype > > used is uint64 but is

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

2021-05-10 Thread vignesh C
On Sun, May 9, 2021 at 6:54 PM Euler Taveira wrote: > > On Sun, May 9, 2021, at 9:37 AM, vignesh C wrote: > > For some of the logical replication messages the data type documented > was not correct, especially for lsn and xid. For lsn actual datatype > used is uint64 but is documented as int64,

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

2021-05-09 Thread Peter Smith
On Sun, May 9, 2021 at 11:13 PM Peter Smith wrote: > > On Sun, May 9, 2021 at 10:38 PM vignesh C wrote: > > > > Hi, > > > > For some of the logical replication messages the data type documented > > was not correct, especially for lsn and xid. For lsn actual datatype > > used is uint64 but is

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

2021-05-09 Thread Euler Taveira
On Sun, May 9, 2021, at 9:37 AM, vignesh C wrote: > For some of the logical replication messages the data type documented > was not correct, especially for lsn and xid. For lsn actual datatype > used is uint64 but is documented as int64, similarly for xid, datatype > used is uint32 but documented

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

2021-05-09 Thread Peter Smith
On Sun, May 9, 2021 at 10:38 PM vignesh C wrote: > > Hi, > > For some of the logical replication messages the data type documented > was not correct, especially for lsn and xid. For lsn actual datatype > used is uint64 but is documented as int64, similarly for xid, datatype > used is uint32 but

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

2021-05-09 Thread vignesh C
Hi, For some of the logical replication messages the data type documented was not correct, especially for lsn and xid. For lsn actual datatype used is uint64 but is documented as int64, similarly for xid, datatype used is uint32 but documented as int32. Attached is a patch which has the fix for