On Wed, Mar 15, 2023 at 11:52 AM Peter Smith <smithpb2...@gmail.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.