RE: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided. - Mailing list pgsql-hackers

Dear Shubham,

Thanks for updating the patch!

> I have added a test case for non-dry-run mode to ensure that
> replication slots, subscriptions, and publications work as expected
> when '--all' is specified. Additionally, I have split the test file
> into two parts:
> 1) Command-line sanity checks – Validates the correctness of option parsing.
> 2) '--all' functionality and behavior – Verifies the health of
> subscriptions and ensures that --all specific scenarios, such as
> non-template databases not being subscribed to, are properly handled.
> This should improve test coverage while keeping the structure manageable.

TBH, I feel your change does not separate the test file into the two parts. ISTM
you just added some validation checks and a test how --all option works.
Ashutosh, does it match your suggestion?

Anyway, here are my comments.

01.
```
+    int            num_rows;
```

I think num_rows is not needed, we can do like:

```
        /* Process the query result */
-       num_rows = PQntuples(res);
-       for (int i = 0; i < num_rows; i++)
+       for (int i = 0; i < PQntuples(res); i+
```
And
```
        /* Error if no databases were found on the source server */
-       if (num_rows == 0)
+       if (num_dbs == 0)
```

02.
```
+    opt.all = false;
```

This must be done after initializing recovery_timeout.

03.
```
+# Set up node S1 as standby linking to node P
+$node_p->backup('backup_3');
+my $node_s1 = PostgreSQL::Test::Cluster->new('node_s1');
```

I do not like the name "S1" because this is the second standby sever
from the node_p.

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.

05.
```
+# Verify replication is working
+$node_p->safe_psql($db1, "INSERT INTO tbl1 VALUES ('replication test');");
+
+$result = $node_s1->safe_psql($db1, "SELECT * FROM tbl1 ORDER BY 1");
+is( $result, qq(first row
+replication test
+second row), "replication successful in $db1");
+
+$node_s1->stop;
 # Run pg_createsubscriber on node S.  --verbose is used twice
 # to show more information.
 command_ok(
```

I'm not sure the part is not needed. This have already been checked in other parts, right?

Best regards,
Hayato Kuroda
FUJITSU LIMITED