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

[Peter Eisentraut]

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.