Dear Shubham,
Thanks for updating the patch quickly!
> > 04.
> > ```
> > +# Verify that only user databases got subscriptions (not template
> > databases)
> > +my @user_dbs = ($db1, $db2);
> > +foreach my $dbname (@user_dbs)
> > +{
> > + my $sub_exists =
> > + $node_s1->safe_psql($dbname, "SELECT count(*) FROM
> pg_subscription;");
> > + is($sub_exists, '3', "Subscription created successfully for
> > $dbname");
> > +}
> > ```
> >
> > Hmm, what do you want to check here? pg_subscription is a global catalog so
> that
> > very loop will see exactly the same content. Also, 'postgres' is also the
> > user
> database.
> > I feel you must ensure that all three databases (postgres, $db1, and $db2)
> > have
> a
> > subscription here.
> >
>
> Fixed.
My point was that the loop does not have meaning because pg_subscription
is a global one. I and Peter considered changes like [1] is needed. It ensures
that each databases have a subscription.
Note: [1] cannot pass the test as-is because $db1 and $db2 contains special
characters. Please escape appropriately.
Other comments are listed in below.
01.
```
+ <term><option>-a</option></term>
+ <term><option>--all-dbs</option></term>
```
Peter's comment [2] does not say that option name should be changed.
The scope of his comment is only in the .c file.
02.
```
+# Set up node S2 as standby linking to node P
+$node_p->backup('backup_3');
+my $node_s2 = PostgreSQL::Test::Cluster->new('node_s2');
```
I feel $node_s should be renamed to $node_s1, then you can use $node_s2.
Note that this change may not be needed based on the comment [3].
We may have to separate the test file.
[1]:
```
# Verify that only user databases got subscriptions (not template databases)
my @user_dbs = ('postgres', $db1, $db2);
foreach my $dbname (@user_dbs)
{
$result =
$node_s2->safe_psql('postgres',
"SELECT count(*) FROM pg_subscription, pg_database
WHERE subdbid = pg_database.oid and datname = '$dbname';");
is($result, '1', "Subscription created successfully for $dbname");
}
```
[2]:
https://www.postgresql.org/message-id/CAHut%2BPsatTfk9-F4JBrX_yYE0QGh4wQiTmvS4%3DdnBxcL%3DAK2HA%40mail.gmail.com
[3]:
https://www.postgresql.org/message-id/CAExHW5vmMs5nZ6%3DXcCYAXMJrhVrsW7hOovyg%2BP%2BT9Pkuc7DykA%40mail.gmail.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
