I have committed your patch to tidy this up. Thanks.
On 26.03.24 06:01, Kyotaro Horiguchi wrote:
> 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.
>