Horton

Re: Allow logical replication to copy tables in binary format

Euler Taveira <euler@eulerto.com>

View thread
From: "Euler Taveira" <euler@eulerto.com>
To: "Amit Kapila" <amit.kapila16@gmail.com>, "Melih Mutlu" <m.melihmutlu@gmail.com>
Cc: "Bharath Rupireddy" <bharath.rupireddyforpostgres@gmail.com>, "Dilip Kumar" <dilipbalaut@gmail.com>, "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
Date: 2023-03-16T02:57:24Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow logical replication to copy tables in binary format.

  2. Common function for percent placeholder replacement

  3. Doc: add XML ID attributes to <sectN> and <varlistentry> tags.

  4. Re-order disable_on_error in tab-complete.

  5. Make subscription tests pass with log_error_verbosity=verbose

  6. Weaken type-OID-matching checks in array_recv and record_recv.

  7. Allow logical replication to transfer data in binary format.

On Wed, Mar 8, 2023, at 11:50 PM, Amit Kapila wrote:
> On Wed, Mar 8, 2023 at 6:17 PM Melih Mutlu <m.melihmutlu@gmail.com> wrote:
> >
> > On 7 Mar 2023 Tue at 04:10 Amit Kapila <amit.kapila16@gmail.com> wrote:
> >>
> >> As per what I could read in this thread, most people prefer to use the
> >> existing binary option rather than inventing a new way (option) to
> >> binary copy in the initial sync phase. Do you agree?
> >
> >
> > I agree.
> > What do you think about the version checks? I removed any kind of check since it’s currently a different option. Should we check publisher version before doing binary copy to ensure that the publisher node supports binary option of COPY command?
> >
> 
> It is not clear to me which version check you wanted to add because we
> seem to have a binary option in COPY from the time prior to logical
> replication. I feel we need a publisher version 14 check as that is
> where we start to support binary mode transfer in logical replication.
> See the check in function libpqrcv_startstreaming().
... then you are breaking existent cases. Even if you have a convincing
argument, you are introducing a behavior change in prior versions (commit
messages should always indicate that you are breaking backward compatibility).

+
+   /*
+    * The binary option for replication is supported since v14
+    */
+   if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 140000 &&
+       MySubscription->binary)
+   {
+       appendStringInfo(&cmd, " WITH (FORMAT binary)");
+       options = lappend(options, makeDefElem("format", (Node *) makeString("binary"), -1));
+   }
+

What are the arguments to support since v14 instead of the to-be-released
version? I read the thread but it is not clear. It was said about the
restrictive nature of this feature and it will be frustrating to see that the
same setup (with the same commands) works with v14 and v15 but it doesn't with
v16. IMO it should be >= 16 and documentation should explain that v14/v15 uses
text format during initial table synchronization even if binary = true.

Should there be a fallback mode (text) if initial table synchronization failed
because of the binary option? Maybe a different setting (auto) to support such
behavior.


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