Re: Identify missing publications from publisher while create/alter subscription.

On Mon, May 3, 2021 at 10:48 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Sun, May 2, 2021 at 10:04 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > Thanks for the comments.
> > The Attached patch has the fixes for the same.
>
> I was reviewing the documentation part, I think in the below paragraph
> we should include validate_publication as well?
>
>        <varlistentry>
>         <term><literal>connect</literal> (<type>boolean</type>)</term>
>         <listitem>
>          <para>
>           Specifies whether the <command>CREATE SUBSCRIPTION</command>
>           should connect to the publisher at all.  Setting this to
>           <literal>false</literal> will change default values of
>           <literal>enabled</literal>, <literal>create_slot</literal> and
>           <literal>copy_data</literal> to <literal>false</literal>.
>          </para>
>
> I will review/test the other parts of the patch and let you know.

I have reviewed it and it mostly looks good to me.  I have some minor
suggestions though.

1.
+/*
+ * Check the specified publication(s) is(are) present in the publisher.
+ */

vs

+
+/*
+ * Connect to the publisher and check if the publications exist.
+ */

I think the formatting of the comments are not uniform.  Some places
we are using "publication(s) is(are)" whereas other places are just
"publications".

2. Add a error case for connect=false and VALIDATE_PUBLICATION = true



--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com