From: Peter Eisentraut <peter@eisentraut.org>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: pg_createsubscriber: creates a new logical replica from a standb
Date: Tue, 26 Mar 2024 08:30:56 +0100
Message-ID: <9609b975-622b-4846-981d-7a5896cd6b51@eisentraut.org> (raw)
In-Reply-To: <20240326.140116.1116279856046587865.horikyota.ntt@gmail.com>
References: <E1roio3-005ZLs-Q3@gemulon.postgresql.org>
<20240326.140116.1116279856046587865.horikyota.ntt@gmail.com>
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.
>
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: peter@eisentraut.org, horikyota.ntt@gmail.com, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: pg_createsubscriber: creates a new logical replica from a standb
In-Reply-To: <9609b975-622b-4846-981d-7a5896cd6b51@eisentraut.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox