On Thu, Mar 16, 2023 at 6:59 PM Melih Mutlu <m.melihmu...@gmail.com> wrote: > > Amit Kapila <amit.kapil...@gmail.com>, 16 Mar 2023 Per, 06:25 tarihinde şunu > yazdı: >> >> On Thu, Mar 16, 2023 at 8:27 AM Euler Taveira <eu...@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? >
I think to reduce the risk of breakage, let's change the check to >=v16. Also, accordingly, update the doc and commit message. -- With Regards, Amit Kapila.