Feedback-ID: i0c21471d:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-811-gb808317eab-fm-20220801.001-gb808317e
Mime-Version: 1.0
Message-Id: <2ebc7ea8-3c46-474e-aea7-5d73ff6165fb@www.fastmail.com>
In-Reply-To: 
 <CAGPVpCQvAziCLknEnygY0v1-KBtg+Om-9JHJYZOnNPKFJPompw@mail.gmail.com>
References: 
 <CAGPVpCQvAziCLknEnygY0v1-KBtg+Om-9JHJYZOnNPKFJPompw@mail.gmail.com>
Date: Wed, 10 Aug 2022 23:03:04 -0300
From: "Euler Taveira" <euler@eulerto.com>
To: "Melih Mutlu" <m.melihmutlu@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Allow logical replication to copy tables in binary format
Content-Type: multipart/alternative;
 boundary=4d0fcfacb8974cc1a3e6a3b3505a133e
Archived-At: 
 <https://www.postgresql.org/message-id/2ebc7ea8-3c46-474e-aea7-5d73ff6165fb%40www.fastmail.com>
Precedence: bulk

--4d0fcfacb8974cc1a3e6a3b3505a133e
Content-Type: text/plain

On Wed, Aug 10, 2022, at 12:03 PM, Melih Mutlu wrote:
> I see that logical replication subscriptions have an option to enable binary [1]. 
> When it's enabled, subscription requests publisher to send data in binary format. 
> But this is only the case for apply phase. In tablesync, tables are still copied as text.
This option could have been included in the commit 9de77b54531; it wasn't.
Maybe it wasn't considered because the initial table synchronization can be a
separate step in your logical replication setup idk. I agree that the binary
option should be available for the initial table synchronization.

> To copy tables, COPY command is used and that command supports copying in binary. So it seemed to me possible to copy in binary for tablesync too.
> I'm not sure if there is a reason to always copy tables in text format. But I couldn't see why not to do it in binary if it's enabled.
The reason to use text format is that it is error prone. There are restrictions
while using the binary format. For example, if your schema has different data
types for a certain column, the copy will fail. Even with such restrictions, I
think it is worth adding it.

> You can find the small patch that only enables binary copy attached.  
I have a few points about your implementation.

* Are we considering to support prior Postgres versions too? These releases
  support binary mode but it could be an unexpected behavior (initial sync in
  binary mode) for a publisher using 14 or 15 and a subscriber using 16. IMO
  you should only allow it for publisher on 16 or later.
* Docs should say that the binary option also applies to initial table
  synchronization and possibly emphasize some of the restrictions.
* Tests. Are the current tests enough? 014_binary.pl.


--
Euler Taveira
EDB   https://www.enterprisedb.com/

--4d0fcfacb8974cc1a3e6a3b3505a133e
Content-Type: text/html
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>On Wed, Aug 10,=
 2022, at 12:03 PM, Melih Mutlu wrote:<br></div><blockquote type=3D"cite=
" id=3D"qt" style=3D""><div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr=
"><div>I see that logical replication subscriptions have an option to en=
able binary [1].&nbsp;<br></div><div>When it's enabled, subscription req=
uests publisher to send data in binary format.&nbsp;<br></div><div>But t=
his is only the case for apply phase. In tablesync, tables are still cop=
ied as text.<br></div></div></div></div></blockquote><div>This option co=
uld have been included in the commit 9de77b54531; it wasn't.<br></div><d=
iv>Maybe it wasn't considered because the initial table synchronization =
can be a<br></div><div>separate step in your logical replication setup i=
dk. I agree that the binary<br></div><div>option should be available for=
 the initial table synchronization.<br></div><div><br></div><blockquote =
type=3D"cite" id=3D"qt" style=3D""><div dir=3D"ltr"><div dir=3D"ltr"><di=
v dir=3D"ltr"><div>To copy tables, COPY command is used and that command=
 supports copying in binary. So it seemed to me possible to copy in bina=
ry for&nbsp;tablesync too.<br></div><div>I'm not sure if there is a reas=
on to always copy tables in text format. But I couldn't see why not to d=
o it in binary if it's enabled.<br></div></div></div></div></blockquote>=
<div>The reason to use text format is that it is error prone. There are =
restrictions<br></div><div>while using the binary format. For example, i=
f your schema has different data<br></div><div>types for a certain colum=
n, the copy will fail. Even with such restrictions, I<br></div><div>thin=
k it is worth adding it.<br></div><div><br></div><blockquote type=3D"cit=
e" id=3D"qt" style=3D""><div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"lt=
r"><div>You can find the small patch that only enables binary copy attac=
hed.&nbsp;&nbsp;<br></div></div></div></div></blockquote><div>I have a f=
ew points about your implementation.<br></div><div><br></div><div>* Are =
we considering to support prior Postgres versions too? These releases<br=
></div><div>&nbsp; support binary mode but it could be an unexpected beh=
avior (initial sync in<br></div><div>&nbsp; binary mode) for a publisher=
 using 14 or 15 and a subscriber using 16. IMO<br></div><div>&nbsp; you =
should only allow it for publisher on 16 or later.<br></div><div>* Docs =
should say that the binary option also applies to initial table<br></div=
><div>&nbsp; synchronization and possibly emphasize some of the restrict=
ions.<br></div><div>* Tests. Are the current tests enough? 014_binary.pl=
.<br></div><div><br></div><div><br></div><div id=3D"sig112409080"><div c=
lass=3D"signature">--<br></div><div class=3D"signature">Euler Taveira<br=
></div><div class=3D"signature">EDB&nbsp;&nbsp; <a href=3D"https://www.e=
nterprisedb.com/">https://www.enterprisedb.com/</a><br></div></div><div>=
<br></div></body></html>
--4d0fcfacb8974cc1a3e6a3b3505a133e--