Hi Amit, On Thu, Sep 24, 2020 at 11:55 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Sep 22, 2020 at 5:15 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Tue, Sep 22, 2020 at 12:02 PM Amit Kapila <amit.kapil...@gmail.com> > > wrote: > > > > > > > > > I am not sure if this suggestion makes it better than what is purposed > > > by Dilip but I think we can declare them in define number order like > > > below: > > > #define LOGICALREP_PROTO_MIN_VERSION_NUM 1 > > > #define LOGICALREP_PROTO_VERSION_NUM 1 > > > #define LOGICALREP_PROTO_STREAM_VERSION_NUM 2 > > > #define LOGICALREP_PROTO_MAX_VERSION_NUM > > > LOGICALREP_PROTO_STREAM_VERSION_NUM > > > > Done this way. > > > > - options.proto.logical.proto_version = LOGICALREP_PROTO_VERSION_NUM; > + options.proto.logical.proto_version = MySubscription->stream ? > + LOGICALREP_PROTO_STREAM_VERSION_NUM : LOGICALREP_PROTO_VERSION_NUM; > > Here, I think instead of using MySubscription->stream, we should use > server/walrecv version number as we used at one place in tablesync.c.
Should subscribers be setting the LR protocol value based on what is the publisher version it is communicating with or should it be set based on whether streaming was enabled or not while creating that subscription? AFAIU if we set this value based on the publisher version (which is lets say >= 14), then it's quite possible that the subscriber will start streaming changes for the in-progress transactions even if the streaming was disabled while creating the subscription, won't it? Please let me know if I am missing something here. Thanks, -- With Regards, Ashutosh Sharma EnterpriseDB:http://www.enterprisedb.com
