Re: pgsql: pg_createsubscriber: creates a new logical replica from a standb

Hello.

This commit added the following error message:

pg_createsubscriber.c: 375
> pg_fatal("could not access directory \"%s\": %s", datadir,
> strerror(errno));

Although other messages use %s with PQresultErrorMessage(), regarding
this specific message, shouldn't we use %m instead of %s + strerror()?
I'm not sure if that would be better.

pg_createsubscriber.c: 687
> pg_log_error("could not obtain database OID: got %d rows, expected %d rows",
> PQntuples(res), 1);
pg_createsubscriber.c: 1652
> pg_log_error("could not obtain subscription OID: got %d rows, expected %d rows",

In these messages, the second %d is always written as "1 rows",
whereas a similar message correctly uses "1 row".

pg_createsubscriber.c: 561
> pg_log_error("could not get system identifier: got %d rows, expected %d row",
> PQntuples(res), 1);

I think it would be better to change the former instances to "%d row",
or to change both to "1 row". I'd like to go the second way but maybe
we should take the first way following our convention.

pg_createsubscriber.c: 923
> pg_log_error("publisher requires wal_level >= logical");

We discussed this message in relation to commit 801792e528, and
decided to quote "logical" to clarify that it is a string literal. I'd
like to follow the conclusion here, too.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center