On Fri, May 7, 2021 at 5:38 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Fri, May 7, 2021 at 11:50 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Thu, May 6, 2021 at 7:22 PM vignesh C <vignesh21@gmail.com> wrote:
> > >
> >
> > Some comments:
> > 1.
> > I don't see any change in pg_dump.c, don't we need to dump this option?
>
> I don't think it is necessary there as the default value of the
> validate_publication is false, so even if the pg_dump has no mention
> of the option, then it is assumed to be false while restoring. Note
> that the validate_publication option is transient (like with other
> options such as create_slot, copy_data) which means it can't be stored
> in pg_subscritpion catalogue. Therefore, user specified value can't be
> fetched once the CREATE/ALTER subscription command is finished. If we
> were to dump the option, we should be storing it in the catalogue,
> which I don't think is necessary. Thoughts?
If we are not storing it in the catalog then it does not need to be dumped.
> > 2.
> > + /* Try to connect to the publisher. */
> > + wrconn = walrcv_connect(sub->conninfo, true, sub->name, &err);
> > + if (!wrconn)
> > + ereport(ERROR,
> > + (errmsg("could not connect to the publisher: %s", err)));
> >
> > Instead of using global wrconn, I think you should use a local variable?
>
> Yeah, we should be using local wrconn, otherwise there can be
> consequences, see the patches at [1]. Thanks for pointing out this.
Right.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com