On Thu, Mar 16, 2023 at 8:27 AM Euler Taveira <euler@eulerto.com> wrote: > > On Wed, Mar 8, 2023, at 11:50 PM, Amit Kapila wrote: > > It is not clear to me which version check you wanted to add because we > seem to have a binary option in COPY from the time prior to logical > replication. I feel we need a publisher version 14 check as that is > where we start to support binary mode transfer in logical replication. > See the check in function libpqrcv_startstreaming(). > > ... then you are breaking existent cases. Even if you have a convincing > argument, you are introducing a behavior change in prior versions (commit > messages should always indicate that you are breaking backward compatibility). > > + > + /* > + * The binary option for replication is supported since v14 > + */ > + if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 140000 && > + MySubscription->binary) > + { > + appendStringInfo(&cmd, " WITH (FORMAT binary)"); > + options = lappend(options, makeDefElem("format", (Node *) makeString("binary"), -1)); > + } > + > > What are the arguments to support since v14 instead of the to-be-released > version? I read the thread but it is not clear. It was said about the > restrictive nature of this feature and it will be frustrating to see that the > same setup (with the same commands) works with v14 and v15 but it doesn't with > v16. >
If the failure has to happen it will anyway happen later when the publisher will be upgraded to v16. The reason for the version checks as v14 was to allow the initial sync from the same version where the binary mode for replication was introduced. However, if we expect failures in the existing setup, I am fine with supporting this for >= v16.
Upgrading the subscriber to v16 and keeping the subscriber in v14 could break existing subscriptions. I don't know how likely such a case is.
I don't have a strong preference on this. What do you think? Should we change it >=v16 or keep it as it is?
