Re: Allow logical replication to copy tables in binary format

On Wed, Mar 15, 2023 at 11:52 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> ======
> src/backend/replication/logical/tablesync.c
>
> 5.
> +
> + /*
> + * If the publisher version is earlier than v14, it COPY command doesn't
> + * support the binary option.
> + */
> + if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 140000 &&
> + MySubscription->binary)
> + {
> + appendStringInfo(&cmd, " WITH (FORMAT binary)");
> + options = lappend(options, makeDefElem("format", (Node *)
> makeString("binary"), -1));
> + }
>
> Sorry, I gave a poor review comment for this previously. Now I have
> revisited all the thread discussions about version checking. I feel
> that some explanation should be given in the code comment so that
> future readers of this code can understand why you decided to use v14
> checking.
>
> Something like this:
>
> SUGGESTION
> If the publisher version is earlier than v14, we use text format COPY.
>

I think this isn't explicit that we supported the binary format since
v14. So, I would prefer my version of the comment as suggested in the
previous email.

--
With Regards,
Amit Kapila.