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

On Mon, May 3, 2021 at 7:59 PM vignesh C <vignesh21@gmail.com> wrote:
> Thanks for the comments, these comments are handle in the v7 patch
> posted in my earlier mail.

Thanks. Some comments on v7 patch:

1) How about "Add publication names from the list to a string."
 instead of
 * Append the list of publication to dest string.

2) How about "Connect to the publisher and see if the given
publication(s) is(are) present."
instead of
 * Connect to the publisher and check if the publication(s) exist.

3) Below comments are unnecessary as the functions/code following them
will tell what the code does.
    /* Verify specified publication(s) exist in the publisher. */
    /* We are done with the remote side, close connection. */

    /* Verify specified publication(s) exist in the publisher. */
    PG_TRY();
    {
        check_publications(wrconn, publications, true);
    }
    PG_FINALLY();
    {
        /* We are done with the remote side, close connection. */
        walrcv_disconnect(wrconn);
    }

4) And also the comment below that's there before check_publications
is unnecessary, as the function name and description would say it all.
/* Verify specified publication(s) exist in the publisher. */

5) A typo -  it is "do not exist"
# Multiple publications does not exist.

6) Should we use "m" specified in all the test cases something like we
do for $stderr =~ m/threads are not supported on this platform/ or
m/replication slot "test_slot" was not created in this database/?
$stderr =~
         /ERROR:  publication "non_existent_pub" does not exist in the
publisher/,

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com