On Tue, May 11, 2021 at 8:06 AM Peter Smith <smithpb2...@gmail.com> wrote:
>
> On Mon, May 10, 2021 at 11:46 PM vignesh C <vignes...@gmail.com> wrote:
> >
> > On Sun, May 9, 2021 at 6:54 PM Euler Taveira <eu...@eulerto.com> 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, similarly for xid, datatype
> > > used is uint32 but documented as int32.
> > > Attached is a patch which has the fix for the same.
> > > Thoughts?
> > >
> > > There was a discussion [1] a few months ago about it. Read the Message
> > > Data
> > > Types definition [2]. It is confusing that an internal data type (int64)
> > > has a
> > > name similar to a generic data type in a protocol definition. As I said
> > > [1] we
> > > should probably inform that that piece of information (LSN) is a
> > > XLogRecPtr.
> > > Since this chapter is intended for developers, I think it is fine to
> > > include
> > > such internal detail.
> >
> > 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?
>
> Adding new message "types" does not seem like a good idea to me. e.g.
> All the message types must be defined by the page [1] so if you add
> new ones then they should also be defined on that page. But then how
> many other places ought to make use of those new types? IMO this
> approach will snowball out of control.
>
> But I am also doubtful there was ever actually a (signed/unsigned)
> problem in the first place. AFAIK the message types like "Int32" etc
> just happen to have a name that "looks" like a C type, but I think
> that is the extent of it. It is simply saying how data bytes are
> transferred on the wire. All the low level C functions [2] always deal
> with unsigned.
>
> ~~
>
> My suggestion would be to restrict your changes to the *description*
> parts of each message. e.g. maybe you could say what C type the bytes
> represent when they come off the wire at the other end - something
> like below.
>
> BEFORE
> Int64
> The final LSN of the transaction.
>
> AFTER
> Int64
> The final LSN (XLogRecPtr) of the transaction
Thanks for the comments, Attached v3 patch has the changes as suggested.
Regards,
Vignesh
From b6d46119b902d3f3007f4b1383774af5859cb615 Mon Sep 17 00:00:00 2001
From: vignesh <vignes...@gmail.com>
Date: Tue, 11 May 2021 20:29:43 +0530
Subject: [PATCH v3] Included the actual datatype used in logical replication
message description.
Included the actual datatype used in logical replication message description.
---
doc/src/sgml/protocol.sgml | 105 ++++++++++++++++++++-----------------
1 file changed, 57 insertions(+), 48 deletions(-)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 2f4dde3beb..ce779b7b1e 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -6378,7 +6378,8 @@ This section describes the detailed format of each logical replication message.
These messages are returned either by the replication slot SQL interface or are
sent by a walsender. In case of a walsender they are encapsulated inside the replication
protocol WAL messages as described in <xref linkend="protocol-replication"/>
-and generally obey same message flow as physical replication.
+and generally obey same message flow as physical replication. The base data types
+used are described in <xref linkend="protocol-message-types"/>.
</para>
<variablelist>
@@ -6407,7 +6408,7 @@ Begin
</term>
<listitem>
<para>
- The final LSN of the transaction.
+ The final LSN (XLogRecPtr) of the transaction.
</para>
</listitem>
</varlistentry>
@@ -6417,8 +6418,8 @@ Begin
</term>
<listitem>
<para>
- Commit timestamp of the transaction. The value is in number
- of microseconds since PostgreSQL epoch (2000-01-01).
+ Commit timestamp (TimestampTz) of the transaction. The value is
+ in number of microseconds since PostgreSQL epoch (2000-01-01).
</para>
</listitem>
</varlistentry>
@@ -6428,7 +6429,7 @@ Begin
</term>
<listitem>
<para>
- Xid of the transaction.
+ Xid (TransactionId) of the transaction.
</para>
</listitem>
</varlistentry>
@@ -6462,8 +6463,9 @@ Message
</term>
<listitem>
<para>
- Xid of the transaction (only present for streamed transactions).
- This field is available since protocol version 2.
+ Xid (TransactionId) of the transaction (only present for
+ streamed transactions). This field is available since protocol
+ version 2.
</para>
</listitem>
</varlistentry>
@@ -6473,8 +6475,8 @@ Message
</term>
<listitem>
<para>
- Flags; Either 0 for no flags or 1 if the logical decoding
- message is transactional.
+ Flags (uint8); Either 0 for no flags or 1 if the logical
+ decoding message is transactional.
</para>
</listitem>
</varlistentry>
@@ -6484,7 +6486,7 @@ Message
</term>
<listitem>
<para>
- The LSN of the logical decoding message.
+ The LSN (XLogRecPtr) of the logical decoding message.
</para>
</listitem>
</varlistentry>
@@ -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>
@@ -6548,7 +6550,7 @@ Commit
</term>
<listitem>
<para>
- The LSN of the commit.
+ The LSN (XLogRecPtr) of the commit.
</para>
</listitem>
</varlistentry>
@@ -6558,7 +6560,7 @@ Commit
</term>
<listitem>
<para>
- The end LSN of the transaction.
+ The end LSN (XLogRecPtr) of the transaction.
</para>
</listitem>
</varlistentry>
@@ -6568,8 +6570,8 @@ Commit
</term>
<listitem>
<para>
- Commit timestamp of the transaction. The value is in number
- of microseconds since PostgreSQL epoch (2000-01-01).
+ Commit timestamp (TimestampTz) of the transaction. The value is
+ in number of microseconds since PostgreSQL epoch (2000-01-01).
</para>
</listitem>
</varlistentry>
@@ -6603,7 +6605,7 @@ Origin
</term>
<listitem>
<para>
- The LSN of the commit on the origin server.
+ The LSN (XLogRecPtr) of the commit on the origin server.
</para>
</listitem>
</varlistentry>
@@ -6652,8 +6654,9 @@ Relation
</term>
<listitem>
<para>
- Xid of the transaction (only present for streamed transactions).
- This field is available since protocol version 2.
+ Xid (TransactionId) of the transaction (only present for
+ streamed transactions). This field is available since
+ protocol version 2.
</para>
</listitem>
</varlistentry>
@@ -6663,7 +6666,7 @@ Relation
</term>
<listitem>
<para>
- ID of the relation.
+ ID (Oid) of the relation.
</para>
</listitem>
</varlistentry>
@@ -6719,8 +6722,8 @@ Relation
</term>
<listitem>
<para>
- Flags for the column. Currently can be either 0 for no flags
- or 1 which marks the column as part of the key.
+ Flags (uint8) for the column. Currently can be either 0 for no
+ flags or 1 which marks the column as part of the key.
</para>
</listitem>
</varlistentry>
@@ -6740,7 +6743,7 @@ Relation
</term>
<listitem>
<para>
- ID of the column's data type.
+ ID (Oid) of the column's data type.
</para>
</listitem>
</varlistentry>
@@ -6784,8 +6787,9 @@ Type
</term>
<listitem>
<para>
- Xid of the transaction (only present for streamed transactions).
- This field is available since protocol version 2.
+ Xid (TransactionId) of the transaction (only present for
+ streamed transactions). This field is available since protocol
+ version 2.
</para>
</listitem>
</varlistentry>
@@ -6795,7 +6799,7 @@ Type
</term>
<listitem>
<para>
- ID of the data type.
+ ID (Oid) of the data type.
</para>
</listitem>
</varlistentry>
@@ -6849,8 +6853,9 @@ Insert
</term>
<listitem>
<para>
- Xid of the transaction (only present for streamed transactions).
- This field is available since protocol version 2.
+ Xid (TransactionId) of the transaction (only present for
+ streamed transactions). This field is available since protocol
+ version 2.
</para>
</listitem>
</varlistentry>
@@ -6860,7 +6865,7 @@ Insert
</term>
<listitem>
<para>
- ID of the relation corresponding to the ID in the relation
+ ID (Oid) of the relation corresponding to the ID in the relation
message.
</para>
</listitem>
@@ -6916,8 +6921,9 @@ Update
</term>
<listitem>
<para>
- Xid of the transaction (only present for streamed transactions).
- This field is available since protocol version 2.
+ Xid (TransactionId) of the transaction (only present for
+ streamed transactions). This field is available since protocol
+ version 2.
</para>
</listitem>
</varlistentry>
@@ -6927,7 +6933,7 @@ Update
</term>
<listitem>
<para>
- ID of the relation corresponding to the ID in the relation
+ ID (Oid) of the relation corresponding to the ID in the relation
message.
</para>
</listitem>
@@ -7030,8 +7036,9 @@ Delete
</term>
<listitem>
<para>
- Xid of the transaction (only present for streamed transactions).
- This field is available since protocol version 2.
+ Xid (TransactionId) of the transaction (only present for
+ streamed transactions). This field is available since protocol
+ version 2.
</para>
</listitem>
</varlistentry>
@@ -7041,7 +7048,7 @@ Delete
</term>
<listitem>
<para>
- ID of the relation corresponding to the ID in the relation
+ ID (Oid) of the relation corresponding to the ID in the relation
message.
</para>
</listitem>
@@ -7119,8 +7126,9 @@ Truncate
</term>
<listitem>
<para>
- Xid of the transaction (only present for streamed transactions).
- This field is available since protocol version 2.
+ Xid (TransactionId) of the transaction (only present for
+ streamed transactions). This field is available since protocol
+ version 2.
</para>
</listitem>
</varlistentry>
@@ -7151,7 +7159,7 @@ Truncate
</term>
<listitem>
<para>
- ID of the relation corresponding to the ID in the relation
+ ID (Oid) of the relation corresponding to the ID in the relation
message. This field is repeated for each relation.
</para>
</listitem>
@@ -7197,7 +7205,7 @@ Stream Start
</term>
<listitem>
<para>
- Xid of the transaction.
+ Xid (TransactionId) of the transaction.
</para>
</listitem>
</varlistentry>
@@ -7266,7 +7274,7 @@ Stream Commit
</term>
<listitem>
<para>
- Xid of the transaction.
+ Xid (TransactionId) of the transaction.
</para>
</listitem>
</varlistentry>
@@ -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>
@@ -7286,7 +7294,7 @@ Stream Commit
</term>
<listitem>
<para>
- The LSN of the commit.
+ The LSN (XLogRecPtr) of the commit.
</para>
</listitem>
</varlistentry>
@@ -7296,7 +7304,7 @@ Stream Commit
</term>
<listitem>
<para>
- The end LSN of the transaction.
+ The end LSN (XLogRecPtr) of the transaction.
</para>
</listitem>
</varlistentry>
@@ -7306,8 +7314,9 @@ Stream Commit
</term>
<listitem>
<para>
- Commit timestamp of the transaction. The value is in number
- of microseconds since PostgreSQL epoch (2000-01-01).
+ Commit timestamp (TimestampTz) of the transaction. The value
+ is in number of microseconds since PostgreSQL epoch
+ (2000-01-01).
</para>
</listitem>
</varlistentry>
@@ -7341,7 +7350,7 @@ Stream Abort
</term>
<listitem>
<para>
- Xid of the transaction.
+ Xid (TransactionId) of the transaction.
</para>
</listitem>
</varlistentry>
@@ -7351,8 +7360,8 @@ Stream Abort
</term>
<listitem>
<para>
- Xid of the subtransaction (will be same as xid of the transaction for top-level
- transactions).
+ Xid (TransactionId) of the subtransaction (will be same as xid
+ of the transaction for top-level transactions).
</para>
</listitem>
</varlistentry>
--
2.25.1
