Error logging messages
Started by Daniel Gustafssonalmost 4 years ago5 messages
When reviewing the patch in "Frontend error logging style" [0]/messages/by-id/1363732.1636496441@sss.pgh.pa.us I noticed that
some messages could do with a little bit of touching up. The original review
was posted and responded to in that thread, but to keep goalposts in place it
was put off until that patch had landed. To avoid this getting buried in that
thread I decided to start a new one with the findings from there. To make
reviewing easier I split the patch around the sorts of changes proposed.
0001: Makes sure that database and file names are printed quoted. This patch
has hunks in contrib and backend as well.
0002: Capitalizes pg_log_error_detail and conversely starts pg_log_error with a
lowercase letter without punctuation.
0003: Extend a few messages with more information to be more consistent with
other messages (and more helpful).
0004: Add pg_log_error() calls on all calls to close in pg_basebackup. Nearly
all had already, and while errors here are likely to be rare, when they do
happen something is really wrong and every bit of information can help
debugging.
0005: Put keywords as parameters in a few pg_dump error messages, to make their
translations reuseable.
--
Daniel Gustafsson https://vmware.com/
Attachments:
0002-pg_log_error-and-pg_log_error_detail-capitalization-.patchapplication/octet-stream; name=0002-pg_log_error-and-pg_log_error_detail-capitalization-.patch; x-unix-mode=0644Download
From 905ea97489d57240f4c49728f0bc6c305776813a Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Wed, 13 Apr 2022 11:12:18 +0200
Subject: [PATCH 2/5] pg_log_error and pg_log_error_detail capitalization and
punctutation
---
src/bin/pg_dump/pg_backup_db.c | 2 +-
src/bin/pg_dump/pg_dump.c | 4 +--
src/bin/pg_dump/pg_dumpall.c | 2 +-
src/bin/pgbench/pgbench.c | 2 +-
src/bin/psql/command.c | 6 ++---
src/bin/psql/common.c | 6 ++---
src/bin/psql/describe.c | 46 +++++++++++++++++-----------------
7 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 89cdbf80e0..9a08ce8968 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -51,7 +51,7 @@ _check_database_version(ArchiveHandle *AH)
remoteversion > AH->public.maxRemoteVersion))
{
pg_log_error("aborting because of server version mismatch");
- pg_log_error_detail("server version: %s; %s version: %s",
+ pg_log_error_detail("Server version: %s; %s version: %s",
remoteversion_str, progname, PG_VERSION);
exit(1);
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 969e2a7a46..3557939f59 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2026,7 +2026,7 @@ dumpTableData_copy(Archive *fout, const void *dcontext)
if (ret == -2)
{
/* copy data transfer failed */
- pg_log_error("Dumping the contents of table \"%s\" failed: PQgetCopyData() failed.", classname);
+ pg_log_error("dumping the contents of table \"%s\" failed: PQgetCopyData() failed", classname);
pg_log_error_detail("Error message from server: %s", PQerrorMessage(conn));
pg_log_error_detail("Command was: %s", q->data);
exit_nicely(1);
@@ -2036,7 +2036,7 @@ dumpTableData_copy(Archive *fout, const void *dcontext)
res = PQgetResult(conn);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- pg_log_error("Dumping the contents of table \"%s\" failed: PQgetResult() failed.", classname);
+ pg_log_error("dumping the contents of table \"%s\" failed: PQgetResult() failed", classname);
pg_log_error_detail("Error message from server: %s", PQerrorMessage(conn));
pg_log_error_detail("Command was: %s", q->data);
exit_nicely(1);
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 6ef3d61421..6848b10b27 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1634,7 +1634,7 @@ connectDatabase(const char *dbname, const char *connection_string,
(server_version / 100) > (my_version / 100)))
{
pg_log_error("aborting because of server version mismatch");
- pg_log_error_detail("server version: %s; %s version: %s",
+ pg_log_error_detail("Server version: %s; %s version: %s",
remoteversion_str, progname, PG_VERSION);
exit_nicely(1);
}
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index e63cea56a1..8585dfc68d 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -7279,7 +7279,7 @@ main(int argc, char **argv)
THREAD_BARRIER_DESTROY(&barrier);
if (exit_code != 0)
- pg_log_error("Run was aborted; the above results are incomplete.");
+ pg_log_error("run was aborted; the above results are incomplete");
return exit_code;
}
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index b51d28780b..7c49d55ff9 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2105,7 +2105,7 @@ exec_command_password(PsqlScanState scan_state, bool active_branch)
}
else if (strcmp(pw1, pw2) != 0)
{
- pg_log_error("Passwords didn't match.");
+ pg_log_error("passwords didn't match");
success = false;
}
else
@@ -3171,7 +3171,7 @@ do_connect(enum trivalue reuse_previous_specification,
/* Complain if we have additional arguments after a connection string. */
if (has_connection_string && (user || host || port))
{
- pg_log_error("Do not give user, host, or port separately when using a connection string");
+ pg_log_error("do not give user, host, or port separately when using a connection string");
return false;
}
@@ -3206,7 +3206,7 @@ do_connect(enum trivalue reuse_previous_specification,
else
{
/* This is reachable after a non-interactive \connect failure */
- pg_log_error("No database connection exists to re-use parameters from");
+ pg_log_error("no database connection exists to re-use parameters from");
return false;
}
}
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index feb1d547d4..0055c6f55e 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -559,7 +559,7 @@ PSQLexec(const char *query)
if (!pset.db)
{
- pg_log_error("You are currently not connected to a database.");
+ pg_log_error("you are currently not connected to a database");
return NULL;
}
@@ -616,7 +616,7 @@ PSQLexecWatch(const char *query, const printQueryOpt *opt, FILE *printQueryFout)
if (!pset.db)
{
- pg_log_error("You are currently not connected to a database.");
+ pg_log_error("you are currently not connected to a database");
return 0;
}
@@ -1121,7 +1121,7 @@ SendQuery(const char *query)
if (!pset.db)
{
- pg_log_error("You are currently not connected to a database.");
+ pg_log_error("you are currently not connected to a database");
goto sendquery_cleanup;
}
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e7377d4583..afe23285c6 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -139,7 +139,7 @@ describeAccessMethods(const char *pattern, bool verbose)
{
char sverbuf[32];
- pg_log_error("The server (version %s) does not support access methods.",
+ pg_log_error("the server (version %s) does not support access methods",
formatPGVersionNumber(pset.sversion, false,
sverbuf, sizeof(sverbuf)));
return true;
@@ -1400,10 +1400,10 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
if (!pset.quiet)
{
if (pattern)
- pg_log_error("Did not find any relation named \"%s\".",
+ pg_log_error("did not find any relation named \"%s\"",
pattern);
else
- pg_log_error("Did not find any relations.");
+ pg_log_error("did not find any relations");
}
PQclear(res);
return false;
@@ -1599,7 +1599,7 @@ describeOneTableDetails(const char *schemaname,
if (PQntuples(res) == 0)
{
if (!pset.quiet)
- pg_log_error("Did not find any relation with OID %s.", oid);
+ pg_log_error("did not find any relation with OID %s", oid);
goto error_return;
}
@@ -3718,13 +3718,13 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
if (PQntuples(res) == 0 && !pset.quiet)
{
if (pattern && pattern2)
- pg_log_error("Did not find any settings for role \"%s\" and database \"%s\".",
+ pg_log_error("did not find any settings for role \"%s\" and database \"%s\"",
pattern, pattern2);
else if (pattern)
- pg_log_error("Did not find any settings for role \"%s\".",
+ pg_log_error("did not find any settings for role \"%s\"",
pattern);
else
- pg_log_error("Did not find any settings.");
+ pg_log_error("did not find any settings");
}
else
{
@@ -3913,10 +3913,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
if (PQntuples(res) == 0 && !pset.quiet)
{
if (pattern)
- pg_log_error("Did not find any relation named \"%s\".",
+ pg_log_error("did not find any relation named \"%s\"",
pattern);
else
- pg_log_error("Did not find any relations.");
+ pg_log_error("did not find any relations");
}
else
{
@@ -3969,7 +3969,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
{
char sverbuf[32];
- pg_log_error("The server (version %s) does not support declarative table partitioning.",
+ pg_log_error("the server (version %s) does not support declarative table partitioning",
formatPGVersionNumber(pset.sversion, false,
sverbuf, sizeof(sverbuf)));
return true;
@@ -4450,7 +4450,7 @@ listEventTriggers(const char *pattern, bool verbose)
{
char sverbuf[32];
- pg_log_error("The server (version %s) does not support event triggers.",
+ pg_log_error("the server (version %s) does not support event triggers",
formatPGVersionNumber(pset.sversion, false,
sverbuf, sizeof(sverbuf)));
return true;
@@ -4524,7 +4524,7 @@ listExtendedStats(const char *pattern)
{
char sverbuf[32];
- pg_log_error("The server (version %s) does not support extended statistics.",
+ pg_log_error("the server (version %s) does not support extended statistics",
formatPGVersionNumber(pset.sversion, false,
sverbuf, sizeof(sverbuf)));
return true;
@@ -5017,10 +5017,10 @@ listTSParsersVerbose(const char *pattern)
if (!pset.quiet)
{
if (pattern)
- pg_log_error("Did not find any text search parser named \"%s\".",
+ pg_log_error("did not find any text search parser named \"%s\"",
pattern);
else
- pg_log_error("Did not find any text search parsers.");
+ pg_log_error("did not find any text search parsers");
}
PQclear(res);
return false;
@@ -5377,10 +5377,10 @@ listTSConfigsVerbose(const char *pattern)
if (!pset.quiet)
{
if (pattern)
- pg_log_error("Did not find any text search configuration named \"%s\".",
+ pg_log_error("did not find any text search configuration named \"%s\"",
pattern);
else
- pg_log_error("Did not find any text search configurations.");
+ pg_log_error("did not find any text search configurations");
}
PQclear(res);
return false;
@@ -5825,10 +5825,10 @@ listExtensionContents(const char *pattern)
if (!pset.quiet)
{
if (pattern)
- pg_log_error("Did not find any extension named \"%s\".",
+ pg_log_error("did not find any extension named \"%s\"",
pattern);
else
- pg_log_error("Did not find any extensions.");
+ pg_log_error("did not find any extensions");
}
PQclear(res);
return false;
@@ -5911,7 +5911,7 @@ listPublications(const char *pattern)
{
char sverbuf[32];
- pg_log_error("The server (version %s) does not support publications.",
+ pg_log_error("the server (version %s) does not support publications",
formatPGVersionNumber(pset.sversion, false,
sverbuf, sizeof(sverbuf)));
return true;
@@ -6033,7 +6033,7 @@ describePublications(const char *pattern)
{
char sverbuf[32];
- pg_log_error("The server (version %s) does not support publications.",
+ pg_log_error("the server (version %s) does not support publications",
formatPGVersionNumber(pset.sversion, false,
sverbuf, sizeof(sverbuf)));
return true;
@@ -6075,10 +6075,10 @@ describePublications(const char *pattern)
if (!pset.quiet)
{
if (pattern)
- pg_log_error("Did not find any publication named \"%s\".",
+ pg_log_error("did not find any publication named \"%s\"",
pattern);
else
- pg_log_error("Did not find any publications.");
+ pg_log_error("did not find any publications");
}
termPQExpBuffer(&buf);
@@ -6211,7 +6211,7 @@ describeSubscriptions(const char *pattern, bool verbose)
{
char sverbuf[32];
- pg_log_error("The server (version %s) does not support subscriptions.",
+ pg_log_error("the server (version %s) does not support subscriptions",
formatPGVersionNumber(pset.sversion, false,
sverbuf, sizeof(sverbuf)));
return true;
--
2.32.0 (Apple Git-132)
0001-Ensure-file-and-database-names-are-quoted.patchapplication/octet-stream; name=0001-Ensure-file-and-database-names-are-quoted.patch; x-unix-mode=0644Download
From 80c7d1cee922540f5012949387961b414759d210 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Wed, 13 Apr 2022 11:10:13 +0200
Subject: [PATCH 1/5] Ensure file and database names are quoted
---
contrib/oid2name/oid2name.c | 2 +-
src/backend/access/transam/xlog.c | 6 +++---
src/bin/pg_waldump/pg_waldump.c | 4 ++--
src/fe_utils/connect_utils.c | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index a62a5eedb1..b64dcc264d 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -329,7 +329,7 @@ sql_conn(struct options *my_opts)
conn = PQconnectdbParams(keywords, values, true);
if (!conn)
- pg_fatal("could not connect to database %s",
+ pg_fatal("could not connect to database \"%s\"",
my_opts->dbname);
if (PQstatus(conn) == CONNECTION_BAD &&
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5eabd32cf6..e1ee5794bb 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2221,7 +2221,7 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible)
errno = save_errno;
ereport(PANIC,
(errcode_for_file_access(),
- errmsg("could not write to log file %s "
+ errmsg("could not write to log file \"%s\" "
"at offset %u, length %zu: %m",
xlogfname, startoffset, nleft)));
}
@@ -3572,7 +3572,7 @@ RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr lastredoptr, XLogRecPtr endptr,
*/
XLogFileName(lastoff, 0, segno, wal_segment_size);
- elog(DEBUG2, "attempting to remove WAL segments older than log file %s",
+ elog(DEBUG2, "attempting to remove WAL segments older than log file \"%s\"",
lastoff);
xldir = AllocateDir(XLOGDIR);
@@ -3649,7 +3649,7 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI)
*/
XLogFileName(switchseg, newTLI, switchLogSegNo, wal_segment_size);
- elog(DEBUG2, "attempting to remove WAL segments newer than log file %s",
+ elog(DEBUG2, "attempting to remove WAL segments newer than log file \"%s\"",
switchseg);
xldir = AllocateDir(XLOGDIR);
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 4f265ef546..3944f7a678 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -375,11 +375,11 @@ WALDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
if (errinfo.wre_errno != 0)
{
errno = errinfo.wre_errno;
- pg_fatal("could not read from file %s, offset %d: %m",
+ pg_fatal("could not read from file \"%s\", offset %d: %m",
fname, errinfo.wre_off);
}
else
- pg_fatal("could not read from file %s, offset %d: read %d of %d",
+ pg_fatal("could not read from file \"%s\", offset %d: read %d of %d",
fname, errinfo.wre_off, errinfo.wre_read,
errinfo.wre_req);
}
diff --git a/src/fe_utils/connect_utils.c b/src/fe_utils/connect_utils.c
index f2e583f9fa..0f7d44bfe6 100644
--- a/src/fe_utils/connect_utils.c
+++ b/src/fe_utils/connect_utils.c
@@ -88,7 +88,7 @@ connectDatabase(const ConnParams *cparams, const char *progname,
conn = PQconnectdbParams(keywords, values, true);
if (!conn)
- pg_fatal("could not connect to database %s: out of memory",
+ pg_fatal("could not connect to database \"%s\": out of memory",
cparams->dbname);
/*
--
2.32.0 (Apple Git-132)
0005-Parameterize-values-to-lessen-translation-burden.patchapplication/octet-stream; name=0005-Parameterize-values-to-lessen-translation-burden.patch; x-unix-mode=0644Download
From 5dbea26c899e15bb3654aae20213f3fca310c2d4 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Wed, 13 Apr 2022 11:31:30 +0200
Subject: [PATCH 5/5] Parameterize values to lessen translation burden
---
src/bin/pg_dump/pg_dump.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3557939f59..815400481f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -11701,8 +11701,8 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
else if (provolatile[0] == PROVOLATILE_STABLE)
appendPQExpBufferStr(q, " STABLE");
else if (provolatile[0] != PROVOLATILE_VOLATILE)
- pg_fatal("unrecognized provolatile value for function \"%s\"",
- finfo->dobj.name);
+ pg_fatal("unrecognized %s value for function \"%s\"",
+ "provolatile", finfo->dobj.name);
}
if (proisstrict[0] == 't')
@@ -11751,8 +11751,8 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
else if (proparallel[0] == PROPARALLEL_RESTRICTED)
appendPQExpBufferStr(q, " PARALLEL RESTRICTED");
else if (proparallel[0] != PROPARALLEL_UNSAFE)
- pg_fatal("unrecognized proparallel value for function \"%s\"",
- finfo->dobj.name);
+ pg_fatal("unrecognized %s value for function \"%s\"",
+ "proparallel", finfo->dobj.name);
}
for (i = 0; i < nconfigitems; i++)
@@ -13461,8 +13461,8 @@ dumpAgg(Archive *fout, const AggInfo *agginfo)
appendPQExpBufferStr(details, ",\n FINALFUNC_MODIFY = READ_WRITE");
break;
default:
- pg_fatal("unrecognized aggfinalmodify value for aggregate \"%s\"",
- agginfo->aggfn.dobj.name);
+ pg_fatal("unrecognized %s value for aggregate \"%s\"",
+ "aggfinalmodify", agginfo->aggfn.dobj.name);
break;
}
}
@@ -13517,8 +13517,8 @@ dumpAgg(Archive *fout, const AggInfo *agginfo)
appendPQExpBufferStr(details, ",\n MFINALFUNC_MODIFY = READ_WRITE");
break;
default:
- pg_fatal("unrecognized aggmfinalmodify value for aggregate \"%s\"",
- agginfo->aggfn.dobj.name);
+ pg_fatal("unrecognized %s value for aggregate \"%s\"",
+ "aggmfinalmodify", agginfo->aggfn.dobj.name);
break;
}
}
@@ -13542,8 +13542,8 @@ dumpAgg(Archive *fout, const AggInfo *agginfo)
else if (proparallel[0] == PROPARALLEL_RESTRICTED)
appendPQExpBufferStr(details, ",\n PARALLEL = restricted");
else if (proparallel[0] != PROPARALLEL_UNSAFE)
- pg_fatal("unrecognized proparallel value for function \"%s\"",
- agginfo->aggfn.dobj.name);
+ pg_fatal("unrecognized %s value for function \"%s\"",
+ "proparallel", agginfo->aggfn.dobj.name);
}
appendPQExpBuffer(delq, "DROP AGGREGATE %s.%s;\n",
--
2.32.0 (Apple Git-132)
0004-Add-errorhandling-on-file-close.patchapplication/octet-stream; name=0004-Add-errorhandling-on-file-close.patch; x-unix-mode=0644Download
From 10891b0d812bb17cc023ee968149755a27a7d565 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Wed, 13 Apr 2022 11:13:43 +0200
Subject: [PATCH 4/5] Add errorhandling on file close
---
src/bin/pg_basebackup/receivelog.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index ad866a7602..e131e9eb2f 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -142,7 +142,9 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
{
pg_log_error("could not fsync existing write-ahead log file \"%s\": %s",
fn, stream->walmethod->getlasterror());
- stream->walmethod->close(f, CLOSE_UNLINK);
+ if (stream->walmethod->close(f, CLOSE_UNLINK) != 0)
+ pg_log_error("could not delete write-ahead log file \"%s\": %s",
+ fn, stream->walmethod->getlasterror());
exit(1);
}
@@ -207,7 +209,9 @@ close_walfile(StreamCtl *stream, XLogRecPtr pos)
{
pg_log_error("could not determine seek position in file \"%s\": %s",
fn, stream->walmethod->getlasterror());
- stream->walmethod->close(walfile, CLOSE_UNLINK);
+ if (stream->walmethod->close(walfile, CLOSE_UNLINK) != 0)
+ pg_log_error("could not delete file \"%s\": %s",
+ fn, stream->walmethod->getlasterror());
walfile = NULL;
pg_free(fn);
@@ -313,7 +317,9 @@ writeTimeLineHistoryFile(StreamCtl *stream, char *filename, char *content)
/*
* If we fail to make the file, delete it to release disk space
*/
- stream->walmethod->close(f, CLOSE_UNLINK);
+ if (stream->walmethod->close(f, CLOSE_UNLINK) != 0)
+ pg_log_error("could not delete timeline history file \"%s\": %s",
+ histfname, stream->walmethod->getlasterror());
return false;
}
--
2.32.0 (Apple Git-132)
0003-Add-additional-information-to-be-more-helpful.patchapplication/octet-stream; name=0003-Add-additional-information-to-be-more-helpful.patch; x-unix-mode=0644Download
From 6932c8171f518271d1359ba72b0fef6e3fc89463 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Wed, 13 Apr 2022 11:13:12 +0200
Subject: [PATCH 3/5] Add additional information to be more helpful
---
src/bin/initdb/initdb.c | 2 +-
src/bin/pg_dump/pg_backup_db.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ab826da650..81ad324040 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2280,7 +2280,7 @@ setup_pgdata(void)
* have embedded spaces.
*/
if (setenv("PGDATA", pg_data, 1) != 0)
- pg_fatal("could not set environment");
+ pg_fatal("could not set PGDATA environment variable");
}
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 9a08ce8968..f71af03d62 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -167,7 +167,7 @@ ConnectDatabase(Archive *AHX,
AH->connection = PQconnectdbParams(keywords, values, true);
if (!AH->connection)
- pg_fatal("could not connect to database");
+ pg_fatal("could not connect to database \"%s\"", cparams->dbname);
if (PQstatus(AH->connection) == CONNECTION_BAD &&
PQconnectionNeedsPassword(AH->connection) &&
--
2.32.0 (Apple Git-132)
Re: Error logging messages
On Wed, Apr 13, 2022 at 01:51:16PM +0200, Daniel Gustafsson wrote:
0001: Makes sure that database and file names are printed quoted. This patch
has hunks in contrib and backend as well.0002: Capitalizes pg_log_error_detail and conversely starts pg_log_error with a
lowercase letter without punctuation.0003: Extend a few messages with more information to be more consistent with
other messages (and more helpful).0005: Put keywords as parameters in a few pg_dump error messages, to make their
translations reuseable.
These look fine.
0004: Add pg_log_error() calls on all calls to close in pg_basebackup. Nearly
all had already, and while errors here are likely to be rare, when they do
happen something is really wrong and every bit of information can help
debugging.
+ if (stream->walmethod->close(f, CLOSE_UNLINK) != 0)
+ pg_log_error("could not delete write-ahead log file \"%s\": %s",
+ fn, stream->walmethod->getlasterror());
With only the file names provided, it is possible to know that this is
a WAL file. Could we switch to a simpler "could not delete file
\"%s\"" instead? Same comment for the history file and the fsync
failure a couple of lines above.
--
MichaelRe: Error logging messages
On 14 Apr 2022, at 09:10, Michael Paquier <michael@paquier.xyz> wrote:
On Wed, Apr 13, 2022 at 01:51:16PM +0200, Daniel Gustafsson wrote:
0001: Makes sure that database and file names are printed quoted. This patch
has hunks in contrib and backend as well.0002: Capitalizes pg_log_error_detail and conversely starts pg_log_error with a
lowercase letter without punctuation.0003: Extend a few messages with more information to be more consistent with
other messages (and more helpful).0005: Put keywords as parameters in a few pg_dump error messages, to make their
translations reuseable.These look fine.
Thanks
0004: Add pg_log_error() calls on all calls to close in pg_basebackup. Nearly
all had already, and while errors here are likely to be rare, when they do
happen something is really wrong and every bit of information can help
debugging.+ if (stream->walmethod->close(f, CLOSE_UNLINK) != 0) + pg_log_error("could not delete write-ahead log file \"%s\": %s", + fn, stream->walmethod->getlasterror()); With only the file names provided, it is possible to know that this is a WAL file. Could we switch to a simpler "could not delete file \"%s\"" instead? Same comment for the history file and the fsync failure a couple of lines above.
I don't have strong opinions, simplifying makes it easier on translators (due
to reuse) and keeping the verbose message may make it easier for users
experiencing problems.
--
Daniel Gustafsson https://vmware.com/
Re: Error logging messages
On 13.04.22 13:51, Daniel Gustafsson wrote:
0002: Capitalizes pg_log_error_detail and conversely starts pg_log_error with a
lowercase letter without punctuation.
I'm having some doubts about some of these changes, especially for
interactive features in psql, where the messages are often use a bit of
a different style. I don't think this kind of thing is an improvement,
for example:
- pg_log_error("You are currently not connected to a database.");
+ pg_log_error("you are currently not connected to a database");If we want to be strict about it, we could change the message to
"not currently connected to a database"
But I do not think just chopping of the punctuation and lower-casing the
first letter of what is after all still a sentence would be an improvement.
Re: Error logging messages
On 14 Apr 2022, at 16:32, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
On 13.04.22 13:51, Daniel Gustafsson wrote:
0002: Capitalizes pg_log_error_detail and conversely starts pg_log_error with a
lowercase letter without punctuation.I'm having some doubts about some of these changes, especially for interactive features in psql, where the messages are often use a bit of a different style. I don't think this kind of thing is an improvement, for example:
- pg_log_error("You are currently not connected to a database."); + pg_log_error("you are currently not connected to a database");
That may also be a bit of Stockholm Syndrome since I prefer the latter error
message, but I see your point.
--
Daniel Gustafsson https://vmware.com/