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

On Tue, Nov 9, 2021 at 9:27 PM vignesh C <vignesh21@gmail.com> wrote:
> Attached v12 version is rebased on top of Head.

Thanks for the patch. Here are some comments on v12:

1) I think ERRCODE_TOO_MANY_ARGUMENTS isn't the right error code, the
ERRCODE_UNDEFINED_OBJECT is more meaningful. Please change.
+ ereport(ERROR,
+ (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+ errmsg_plural("publication %s does not exist in the publisher",
+    "publications %s do not exist in the publisher",

The existing code using
        ereport(ERROR,
                (errcode(ERRCODE_UNDEFINED_OBJECT),
                 errmsg("subscription \"%s\" does not exist", subname)));

2) Typo: It is "One of the specified publications exists."
+# One of the specified publication exist.

3) I think we can remove the test case "+# Specified publication does
not exist." because the "+# One of the specified publication exist."
covers the code.

4) Do we need the below test case? Even with refresh = false, it does
call connect_and_check_pubs() right? Please remove it.
+# Specified publication does not exist with refresh = false.
+($ret, $stdout, $stderr) = $node_subscriber->psql('postgres',
+       "ALTER SUBSCRIPTION mysub1 SET PUBLICATION non_existent_pub
WITH (REFRESH = FALSE, VALIDATE_PUBLICATION = TRUE)"
+);
+ok( $stderr =~
+         m/ERROR:  publication "non_existent_pub" does not exist in
the publisher/,
+       "Alter subscription for non existent publication fails");
+

5) Change the test case names to different ones instead of the same.
Have something like:
"Create subscription fails with single non-existent publication");
"Create subscription fails with multiple non-existent publications");
"Create subscription fails with mutually exclusive options");
"Alter subscription add publication fails with non-existent publication");
"Alter subscription set publication fails with non-existent publication");
"Alter subscription set publication fails with connection to a
non-existent database");

Unnecessary test cases would add up to the "make check-world" times,
please remove them.

Regards,
Bharath Rupireddy.