Re: refactor some protocol message sending in walsender and basebackup

2022-07-06 Thread Peter Eisentraut

On 01.07.22 23:36, Nathan Bossart wrote:

On Thu, Jun 23, 2022 at 04:36:36PM +0200, Peter Eisentraut wrote:

Some places in walsender.c and basebackup_copy.c open-code the sending of
RowDescription and DataRow protocol messages.  But there are already more
compact and robust solutions available for this, using DestRemoteSimple and
associated machinery, already in use in walsender.c.

The attached patches 0001 and 0002 are tiny bug fixes I found during this.

Patches 0003 and 0004 are the main refactorings.  They should probably be
combined into one patch eventually, but this way the treatment of
RowDescription and DataRow is presented separately.


All 4 patches look reasonable to me.


All committed now, thanks.

(I cleaned up the 0004 patch a bit more; there was some junk left in the 
posted patch.)





Re: refactor some protocol message sending in walsender and basebackup

2022-07-01 Thread Nathan Bossart
On Thu, Jun 23, 2022 at 04:36:36PM +0200, Peter Eisentraut wrote:
> Some places in walsender.c and basebackup_copy.c open-code the sending of
> RowDescription and DataRow protocol messages.  But there are already more
> compact and robust solutions available for this, using DestRemoteSimple and
> associated machinery, already in use in walsender.c.
> 
> The attached patches 0001 and 0002 are tiny bug fixes I found during this.
> 
> Patches 0003 and 0004 are the main refactorings.  They should probably be
> combined into one patch eventually, but this way the treatment of
> RowDescription and DataRow is presented separately.

All 4 patches look reasonable to me.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




refactor some protocol message sending in walsender and basebackup

2022-06-23 Thread Peter Eisentraut
Some places in walsender.c and basebackup_copy.c open-code the sending 
of RowDescription and DataRow protocol messages.  But there are already 
more compact and robust solutions available for this, using 
DestRemoteSimple and associated machinery, already in use in walsender.c.


The attached patches 0001 and 0002 are tiny bug fixes I found during this.

Patches 0003 and 0004 are the main refactorings.  They should probably 
be combined into one patch eventually, but this way the treatment of 
RowDescription and DataRow is presented separately.From 069b9896832412555470e30b481df8cc1e6bebec Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 19 May 2022 12:26:45 +0200
Subject: [PATCH 1/4] Fix attlen in RowDescription of BASE_BACKUP response

Should be 8 for int8, not -1.
---
 src/backend/replication/basebackup_copy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/replication/basebackup_copy.c 
b/src/backend/replication/basebackup_copy.c
index cabb077240..1eed9d8c3f 100644
--- a/src/backend/replication/basebackup_copy.c
+++ b/src/backend/replication/basebackup_copy.c
@@ -361,7 +361,7 @@ SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli)
 * would not be wide enough for this, as TimeLineID is unsigned.
 */
pq_sendint32(, INT8OID);/* type oid */
-   pq_sendint16(, -1);
+   pq_sendint16(, 8);
pq_sendint32(, 0);
pq_sendint16(, 0);
pq_endmessage();

base-commit: ac0e2d387a044faed310cbfe2fae78ecb0f6a4b6
-- 
2.36.1

From 045730e92bb67806f31fee033e1f87c69c8ae08e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 23 Jun 2022 10:56:55 +0200
Subject: [PATCH 2/4] Change timeline field of IDENTIFY_SYSTEM to int8

It was int4, but in the other replication commands, timelines are
returned as int8.
---
 doc/src/sgml/protocol.sgml  | 2 +-
 src/backend/replication/walsender.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index a94743b587..c0b89a3c01 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1838,7 +1838,7 @@ Streaming Replication Protocol

 

-timeline (int4)
+timeline (int8)
 
  
   Current timeline ID. Also useful to check that the standby is
diff --git a/src/backend/replication/walsender.c 
b/src/backend/replication/walsender.c
index e42671722a..fa60c92e13 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -444,7 +444,7 @@ IdentifySystem(void)
TupleDescInitBuiltinEntry(tupdesc, (AttrNumber) 1, "systemid",
  TEXTOID, -1, 0);
TupleDescInitBuiltinEntry(tupdesc, (AttrNumber) 2, "timeline",
- INT4OID, -1, 0);
+ INT8OID, -1, 0);
TupleDescInitBuiltinEntry(tupdesc, (AttrNumber) 3, "xlogpos",
  TEXTOID, -1, 0);
TupleDescInitBuiltinEntry(tupdesc, (AttrNumber) 4, "dbname",
-- 
2.36.1

From 9f30600e54e79a6d1e0ca4feda1511d90092148a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 23 Jun 2022 10:55:09 +0200
Subject: [PATCH 3/4] Refactor sending of RowDescription messages in
 replication protocol

Some routines open-coded the construction of RowDescription messages.
Instead, we have support for doing this using tuple descriptors and
DestRemoteSimple, so use that instead.
---
 src/backend/access/common/tupdesc.c   |  9 +++
 src/backend/replication/basebackup_copy.c | 74 +++
 src/backend/replication/walsender.c   | 29 +++--
 3 files changed, 40 insertions(+), 72 deletions(-)

diff --git a/src/backend/access/common/tupdesc.c 
b/src/backend/access/common/tupdesc.c
index 9f41b1e854..d6fb261e20 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -739,6 +739,15 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
att->attcollation = InvalidOid;
break;
 
+   case OIDOID:
+   att->attlen = 4;
+   att->attbyval = true;
+   att->attalign = TYPALIGN_INT;
+   att->attstorage = TYPSTORAGE_PLAIN;
+   att->attcompression = InvalidCompressionMethod;
+   att->attcollation = InvalidOid;
+   break;
+
default:
elog(ERROR, "unsupported type %u", oidtypeid);
}
diff --git a/src/backend/replication/basebackup_copy.c 
b/src/backend/replication/basebackup_copy.c
index 1eed9d8c3f..df0471a7a4 100644
--- a/src/backend/replication/basebackup_copy.c
+++ b/src/backend/replication/basebackup_copy.c
@@ -25,11 +25,13 @@
  */
 #include "postgres.h"
 
+#include