Re: Allow logical replication to copy tables in binary format

Peter Smith <smithpb2250@gmail.com>

From: Peter Smith <smithpb2250@gmail.com>
To: Melih Mutlu <m.melihmutlu@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, Euler Taveira <euler@eulerto.com>, 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>
Date: 2023-03-18T09:41:23Z
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 Fri, Mar 17, 2023 at 11:25 PM Melih Mutlu <m.melihmutlu@gmail.com> wrote:
>
> Hi,
>
> Sharing v17.
>
> Amit Kapila <amit.kapila16@gmail.com>, 17 Mar 2023 Cum, 03:02 tarihinde şunu yazdı:
>>
>> I think to reduce the risk of breakage, let's change the check to
>> >=v16. Also, accordingly, update the doc and commit message.
>
>
> Done.
>

Here are my review comments for v17-0001


======
Commit message

1.
Binary copy is supported for v16 or later.

~

As written that's very general and not quite correct. E.g. COPY ...
WITH (FORMAT binary) has been available for a long time. IMO that
commit message sentence ought to be more specific.

SUGGESTION
Binary copy for logical replication table synchronization is supported
only when both publisher and subscriber are v16 or later.

======
src/backend/replication/logical/tablesync.c

2.
@@ -1168,6 +1170,15 @@ copy_table(Relation rel)

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


Logical replication binary mode was introduced in v14, so the old
comment ("The binary option for replication is supported since v14")
was correct. Unfortunately, after changing the code check to 16000, I
think the new comment ("The binary option for replication is supported
since v16") became incorrect, and so it needs some rewording. Maybe it
should say something like below:

SUGGESTION
If the publisher is v16 or later, then any initial table
synchronization will use the same format as specified by the
subscription binary mode. If the publisher is before v16, then any
initial table synchronization will use text format regardless of the
subscription binary mode.

------
Kind Regards,
Peter Smith.
Fujitsu Australia