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
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index b8f8269340..a6e0986670 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -372,8 +372,7 @@ check_data_directory(const char *datadir)
if (errno == ENOENT)
pg_fatal("data directory \"%s\" does not exist", datadir);
else
- pg_fatal("could not access directory \"%s\": %s", datadir,
- strerror(errno));
+ pg_fatal("could not access directory \"%s\": %m", datadir);
}
snprintf(versionfile, MAXPGPATH, "%s/PG_VERSION", datadir);
@@ -684,7 +683,7 @@ generate_object_name(PGconn *conn)
if (PQntuples(res) != 1)
{
- pg_log_error("could not obtain database OID: got %d rows, expected %d rows",
+ pg_log_error("could not obtain database OID: got %d rows, expected %d row",
PQntuples(res), 1);
disconnect_database(conn, true);
}
@@ -920,7 +919,7 @@ check_publisher(const struct LogicalRepInfo *dbinfo)
if (strcmp(wal_level, "logical") != 0)
{
- pg_log_error("publisher requires wal_level >= logical");
+ pg_log_error("publisher requires wal_level >= \"logical\"");
failed = true;
}
@@ -1649,7 +1648,7 @@ set_replication_progress(PGconn *conn, const struct LogicalRepInfo *dbinfo, cons
if (PQntuples(res) != 1 && !dry_run)
{
- pg_log_error("could not obtain subscription OID: got %d rows, expected %d rows",
+ pg_log_error("could not obtain subscription OID: got %d rows, expected %d row",
PQntuples(res), 1);
disconnect_database(conn, true);
}
