Horton

RE: Allow logical replication to copy tables in binary format

Takamichi Osumi (Fujitsu) <osumi.takamichi@fujitsu.com>

View thread
From: "Takamichi Osumi (Fujitsu)" <osumi.takamichi@fujitsu.com>
To: 'Melih Mutlu' <m.melihmutlu@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, Euler Taveira <euler@eulerto.com>, Amit Kapila <amit.kapila16@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2023-02-26T22:58:26Z
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 Monday, February 20, 2023 8:47 PM Melih Mutlu <m.melihmutlu@gmail.com> wrote:
> Thanks for letting me know. 
> Attached the fixed version of the patch.
Hi, Melih


Thanks for updating the patch. Minor comments on v9.

(1) commit message

"The patch introduces a new parameter, copy_format, to CREATE SUBSCRIPTION to allow to choose
the format used in initial table synchronization."

This patch introduces the new parameter not only to CREATE SUBSCRIPTION and ALTER SUBSCRIPTION, then this description should be more general, something like below.

"The patch introduces a new parameter, copy_format, as subscription option to
allow user to choose the format of initial table synchronization."

(2) copy_table

We don't need to check the publisher's version below.

+
+       /* If the publisher is v16 or later, specify the format to copy data. */
+       if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 160000)
+       {
+               char *format = MySubscription->copyformat == LOGICALREP_COPY_AS_BINARY ? "binary" : "text";
+               appendStringInfo(&cmd, "  WITH (FORMAT %s)", format);
+               options = lappend(options, makeDefElem("format", (Node *) makeString(format), -1));
+       }
+

We don't have this kind of check for "binary" option and it seems this is user's responsibility to avoid any errors during replication. If we want to add this kind of check, then we can add checks for both "binary" and "copy_format" option together as an independent patch.

(3) subscription.sql/out

The other existing other subscription options check the invalid input for newly created option (e.g. "foo" for disable_on_error,  streaming mode). So, I think we can add this type of test for this feature.



Best Regards,
	Takamichi Osumi