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?
> 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.
[1] - https://www.postgresql.org/message-id/CAHut%2BPuSwWmmeK%2Bfe6E2duep8588Jk82XXH73nE4dUxwDNkNUg%40mail.gmail.com
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com