RE: Allow logical replication to copy tables in binary format

On Mon, Nov 14, 2022 8:08 PM Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com> wrote:
>
> Attached patch with updated version of this patch.

Thanks for your patch.

I tried to do a performance test for this patch, the result looks good to me.
(The steps are similar to what Melih shared [1].)

The time to synchronize about 1GB data in binary (the average of the middle
eight was taken):
HEAD: 16.854 s
Patched: 6.805 s

Besides, here are some comments.

1.
+# Binary enabled subscription should fail
+$node_subscriber_binary->wait_for_log("ERROR: insufficient data left in message");

Should it be changed to "ERROR: ( [A-Z0-9]+:)? ", like other subscription tests.

2.
+# Binary disabled subscription should succeed
+$node_publisher->wait_for_catchup('tap_sub');

If we want to wait for table synchronization to finish, should we call
wait_for_subscription_sync()?

3.
I also think it might be better to support copy binary only for publishers of
v16 or later. Do you plan to implement it in the patch?

[1] https://www.postgresql.org/message-id/CAGPVpCQEKDVKQPf6OFQ-9WiRYB1YRejm--YJTuwgzuvj1LEJ2A%40mail.gmail.com

Regards,
Shi yu