pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedredundant error messages
5+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2020-11-07 16eadc4695 42c088be18 468bed27ce 8096be2617 24bbdaa6fe f3ad4fddfa 6be725e701 Fix redundant error messages in client tools
* redundant error messages
@ 2020-11-05 12:27 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 2 replies; 5+ messages in thread
From: Peter Eisentraut @ 2020-11-05 12:27 UTC (permalink / raw)
To: pgsql-hackers
A few client tools duplicate error messages already provided by libpq,
such as
pg_rewind: fatal: could not connect to server: could not connect to
server: No such file or directory
pg_basebackup: error: could not connect to server: could not connect to
server: No such file or directory
psql: error: could not connect to server: could not connect to server:
No such file or directory
The psql case is actually a regression introduced in PG12, but the other
two appear to be ancient.
Other client tools provide a different error message so in aggregate it
looks like this:
createdb: error: could not connect to database template1: could not
connect to server: No such file or directory
The attached patch removes the redundant message from the client tools.
I suppose it's a bit dubious because there is no guarantee what the
level of detail the message supplied by libpq has. But I think these
few cases are not particularly hard to keep in sync.
Attachments:
0001-Fix-redundant-error-messages-in-client-tools.patch (text/plain, 2.0K) download
From a2c1e26ceff05ec6ebbdb9cf74a3171ebf969911 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 5 Nov 2020 13:24:47 +0100
Subject: [PATCH] Fix redundant error messages in client tools
A few client tools duplicate error messages already provided by libpq.
---
src/bin/pg_basebackup/streamutil.c | 5 ++---
src/bin/pg_rewind/pg_rewind.c | 3 +--
src/bin/psql/startup.c | 2 +-
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index be653ebb2d..5c536eb5fd 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -183,7 +183,7 @@ GetConnection(void)
*/
if (!tmpconn)
{
- pg_log_error("could not connect to server");
+ pg_log_error("out of memory");
exit(1);
}
@@ -200,8 +200,7 @@ GetConnection(void)
if (PQstatus(tmpconn) != CONNECTION_OK)
{
- pg_log_error("could not connect to server: %s",
- PQerrorMessage(tmpconn));
+ pg_log_error("%s", PQerrorMessage(tmpconn));
PQfinish(tmpconn);
free(values);
free(keywords);
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 421a45ef5b..52e3fc40e8 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -282,8 +282,7 @@ main(int argc, char **argv)
conn = PQconnectdb(connstr_source);
if (PQstatus(conn) == CONNECTION_BAD)
- pg_fatal("could not connect to server: %s",
- PQerrorMessage(conn));
+ pg_fatal("%s", PQerrorMessage(conn));
if (showprogress)
pg_log_info("connected to server");
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index e8d35a108f..586fcb3366 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -295,7 +295,7 @@ main(int argc, char *argv[])
if (PQstatus(pset.db) == CONNECTION_BAD)
{
- pg_log_error("could not connect to server: %s", PQerrorMessage(pset.db));
+ pg_log_error("%s", PQerrorMessage(pset.db));
PQfinish(pset.db);
exit(EXIT_BADCONN);
}
--
2.28.0
* Re: redundant error messages
@ 2020-11-05 13:34 Euler Taveira <euler.taveira@2ndquadrant.com>
parent: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
1 sibling, 1 reply; 5+ messages in thread
From: Euler Taveira @ 2020-11-05 13:34 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; +Cc: pgsql-hackers
On Thu, 5 Nov 2020 at 09:27, Peter Eisentraut <
peter.eisentraut@enterprisedb.com> wrote:
> A few client tools duplicate error messages already provided by libpq,
> such as
>
> pg_rewind: fatal: could not connect to server: could not connect to
> server: No such file or directory
>
> Good catch!
> Other client tools provide a different error message so in aggregate it
> looks like this:
>
> createdb: error: could not connect to database template1: could not
> connect to server: No such file or directory
>
> Is the database name important for this message? You should inform which
database you want to connect for all client tools except pg_dumpall. Hence,
you
already know which database has the connection problem. IMO the pg_dumpall
message should inform the database name. My suggestion is:
if (fail_on_error)
{
pg_log_error("database \"%s\": %s",
dbname, PQerrorMessage(conn));
exit_nicely(1);
}
and remove the redundant 'could not connect to database %s' from
scripts/common.c.
--
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: redundant error messages
@ 2020-11-05 14:49 Isaac Morland <isaac.morland@gmail.com>
parent: Euler Taveira <euler.taveira@2ndquadrant.com>
0 siblings, 1 reply; 5+ messages in thread
From: Isaac Morland @ 2020-11-05 14:49 UTC (permalink / raw)
To: Euler Taveira <euler.taveira@2ndquadrant.com>; +Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-hackers
On Thu, 5 Nov 2020 at 08:34, Euler Taveira <euler.taveira@2ndquadrant.com>
wrote:
Is the database name important for this message? You should inform which
> database you want to connect for all client tools except pg_dumpall.
> Hence, you
> already know which database has the connection problem. IMO the pg_dumpall
> message should inform the database name. My suggestion is:
>
In principle, the client knows the database name. In practice, if it's
coming from PGDATABASE or via a service configuration, one may be confused
about the database; having the error message be explicit will avoid many
problems. I can easily imagine that "unable to connect to database" would
be mystifying, whereas "unable to connect to database foo" would elicit the
response, "wait, I'm trying to connect to what now?" leading much more
quickly to a resolution.
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: redundant error messages
@ 2020-11-05 15:53 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: Isaac Morland <isaac.morland@gmail.com>
0 siblings, 0 replies; 5+ messages in thread
From: Alvaro Herrera @ 2020-11-05 15:53 UTC (permalink / raw)
To: Isaac Morland <isaac.morland@gmail.com>; +Cc: Euler Taveira <euler.taveira@2ndquadrant.com>; Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-hackers
On 2020-Nov-05, Isaac Morland wrote:
> In principle, the client knows the database name. In practice, if it's
> coming from PGDATABASE or via a service configuration, one may be confused
> about the database; having the error message be explicit will avoid many
> problems. I can easily imagine that "unable to connect to database" would
> be mystifying, whereas "unable to connect to database foo" would elicit the
> response, "wait, I'm trying to connect to what now?" leading much more
> quickly to a resolution.
Also consider cases like running something via cron, where the person
reading the error output does not necessarily know what command is being
run: it might be hidden inside a script. It's often very helpful to
have object names in error messages, even if for the normal usage it
seems that the object being operated on is very obvious by just looking
at the command.
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: redundant error messages
@ 2020-11-07 22:15 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
parent: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
1 sibling, 0 replies; 5+ messages in thread
From: Peter Eisentraut @ 2020-11-07 22:15 UTC (permalink / raw)
To: pgsql-hackers
On 2020-11-05 13:27, Peter Eisentraut wrote:
> A few client tools duplicate error messages already provided by libpq,
> such as
>
> pg_rewind: fatal: could not connect to server: could not connect to
> server: No such file or directory
>
> pg_basebackup: error: could not connect to server: could not connect to
> server: No such file or directory
>
> psql: error: could not connect to server: could not connect to server:
> No such file or directory
>
> The psql case is actually a regression introduced in PG12, but the other
> two appear to be ancient.
I have committed fixes for these.
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2020-11-07 22:15 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05 12:27 redundant error messages Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2020-11-05 13:34 ` Euler Taveira <euler.taveira@2ndquadrant.com>
2020-11-05 14:49 ` Isaac Morland <isaac.morland@gmail.com>
2020-11-05 15:53 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-11-07 22:15 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
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