Dear Shubham,
> Fixed the given comments. The attached patch at [1] contains the
> suggested changes.
Thanks for updates. I registered the thread to the commitfest entry [1].
Few comments.
01. fetch_source_databases
```
+ const char *query = "SELECT datname FROM pg_database WHERE datistemplate = false";
```
You told given comments were fixed, but you ignored this. Please address it
or clarify the reason why you didn't. My comment was:
> We should verify the attribute datallowconn, which indicates we can connect to the
> database. If it is false, no one would be able to connect to the db and define anything.
02. fetch_source_databases
```
+ /* Check for connection errors */
+ if (PQstatus(conn) != CONNECTION_OK)
+ {
+ pg_log_error("connection to the source server failed: %s", PQerrorMessage(conn));
+ disconnect_database(conn, false);
+ exit(1);
+ }
```
connect_database() does the error handling, no need to do manually.
03. fetch_source_databases
```
+ pg_log_error("failed to execute query on the source server: %s", PQerrorMessage(conn));
```
I feel the error message is too general. Also, PQerrorMessage() is not suitable for getting error messages
generated by the query. How about:
pg_log_error("could not obtain a list of databases: %s", PQresultErrorMessage());
04. fetch_source_databases
```
+ /* Error if no databases were found on the source server */
+ if (num_rows == 0)
+ {
+ pg_log_error("no databases found on the source server");
+ pg_log_error_hint("Ensure that there are user-created databases on the source server.");
+ PQclear(res);
+ disconnect_database(conn, false);
+ exit(1);
+ }
```
Can you clarify when this happens?
05. main
```
case 'd':
+ if (opt.all_databases)
+ {
+ pg_log_error("--database cannot be used with --all-databases");
+ exit(1);
+
```
I think this erroring is not enough. getopt_long() receives options with the specified ordering, thus
-d can be accepted if the -a is specified latter.
(Same thing can be said for 'P', '--replslot' and '--subscription'.)
pg_createsubscriber ... -a -d postgres -> can be rejected,
pg_createsubscriber ... -d postgres -a -> cannot be rejected.
To avoid the issue, you must 1) add if-statements in 'a' case or 2) do validation outside of the
switch-case. I prefer 2.
[1]: https://commitfest.postgresql.org/52/5566/
Best regards,
Hayato Kuroda
FUJITSU LIMITED
