pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: strange error reporting
Date: Thu, 21 Jan 2021 17:03:09 -0500
Message-ID: <1094524.1611266589@sss.pgh.pa.us> (raw )
In-Reply-To: <798280.1611192827@sss.pgh.pa.us >
References: <CA+TgmobssJ6rS22dspWnu-oDxXevGmhMD8VcRBjmj-b9UDqRjw@mail.gmail.com >
<717401.1611163169@sss.pgh.pa.us >
<CA+Tgmoas37XHT48KjEb0zACCTxSTkba_VP-BDQAft-Mj5_5z_Q@mail.gmail.com >
<718535.1611164867@sss.pgh.pa.us >
<CA+Tgmobcfsh9jVdgiF+cUwtmbrdiLE8F9mt7a2J4yyO-ix1G1A@mail.gmail.com >
<798280.1611192827@sss.pgh.pa.us >
I wrote:
> If I don't hear any other opinions, I'll change these messages to
> "connection to server at socket \"%s\" failed: "
> "connection to server at \"%s\" (%s), port %s failed: "
Done. Also, here is a patch to remove the redundant-seeming prefixes
from our reports of connection failures. My feeling that this is the
right thing was greatly increased when I noticed that psql, as well as
a few other programs, already did it like this. (I still favor
Alvaro's patch for the back branches, though.)
regards, tom lane
Attachments:
make-connection-failure-messages-less-redundant.patch (text/x-diff, 13.4K) download
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index 5a884e2904..65cce49993 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -347,8 +347,7 @@ sql_conn(struct options *my_opts)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) == CONNECTION_BAD)
{
- pg_log_error("could not connect to database %s: %s",
- my_opts->dbname, PQerrorMessage(conn));
+ pg_log_error("%s", PQerrorMessage(conn));
PQfinish(conn);
exit(1);
}
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index cdc2c02b8e..dcb95c4320 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -124,8 +124,7 @@ vacuumlo(const char *database, const struct _param *param)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) == CONNECTION_BAD)
{
- pg_log_error("connection to database \"%s\" failed: %s",
- database, PQerrorMessage(conn));
+ pg_log_error("%s", PQerrorMessage(conn));
PQfinish(conn);
return -1;
}
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 2bb3bf77e4..5f5faaa0b7 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -6837,8 +6837,8 @@ main(void)
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ /* PQerrorMessage's result includes a trailing newline */
+ fprintf(stderr, "%s", PQerrorMessage(conn));
PQfinish(conn);
return 1;
}
@@ -8296,8 +8296,7 @@ main(int argc, char **argv)
/* Check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
@@ -8466,8 +8465,7 @@ main(int argc, char **argv)
/* Check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
@@ -8694,8 +8692,7 @@ main(int argc, char **argv)
/* Check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
diff --git a/doc/src/sgml/lobj.sgml b/doc/src/sgml/lobj.sgml
index 413eda50af..6d46da42e2 100644
--- a/doc/src/sgml/lobj.sgml
+++ b/doc/src/sgml/lobj.sgml
@@ -939,8 +939,7 @@ main(int argc, char **argv)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 5ba43441f5..2856c16e85 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -188,12 +188,10 @@ ConnectDatabase(Archive *AHX,
if (PQstatus(AH->connection) == CONNECTION_BAD)
{
if (isReconnect)
- fatal("reconnection to database \"%s\" failed: %s",
- PQdb(AH->connection) ? PQdb(AH->connection) : "",
+ fatal("reconnection failed: %s",
PQerrorMessage(AH->connection));
else
- fatal("connection to database \"%s\" failed: %s",
- PQdb(AH->connection) ? PQdb(AH->connection) : "",
+ fatal("%s",
PQerrorMessage(AH->connection));
}
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 85d08ad660..007a3d0f9a 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1768,8 +1768,7 @@ connectDatabase(const char *dbname, const char *connection_string,
{
if (fail_on_error)
{
- pg_log_error("could not connect to database \"%s\": %s",
- dbname, PQerrorMessage(conn));
+ pg_log_error("%s", PQerrorMessage(conn));
exit_nicely(1);
}
else
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index a9bbb80e63..798884da36 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -3460,7 +3460,7 @@ foreach my $db (sort keys %create_sql)
command_fails_like(
[ 'pg_dump', '-p', "$port", 'qqq' ],
- qr/pg_dump: error: connection to database "qqq" failed: connection to server .* failed: FATAL: database "qqq" does not exist/,
+ qr/pg_dump: error: connection to server .* failed: FATAL: database "qqq" does not exist/,
'connecting to a non-existent database');
#########################################
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index 31b1425202..7fed0ae108 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -30,8 +30,7 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
if (conn == NULL || PQstatus(conn) != CONNECTION_OK)
{
- pg_log(PG_REPORT, "connection to database failed: %s",
- PQerrorMessage(conn));
+ pg_log(PG_REPORT, "%s", PQerrorMessage(conn));
if (conn)
PQfinish(conn);
@@ -50,6 +49,8 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
* get_db_conn()
*
* get database connection, using named database + standard params for cluster
+ *
+ * Caller must check for connection failure!
*/
static PGconn *
get_db_conn(ClusterInfo *cluster, const char *db_name)
@@ -294,8 +295,7 @@ start_postmaster(ClusterInfo *cluster, bool report_and_exit_on_error)
if ((conn = get_db_conn(cluster, "template1")) == NULL ||
PQstatus(conn) != CONNECTION_OK)
{
- pg_log(PG_REPORT, "\nconnection to database failed: %s",
- PQerrorMessage(conn));
+ pg_log(PG_REPORT, "\n%s", PQerrorMessage(conn));
if (conn)
PQfinish(conn);
if (cluster == &old_cluster)
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index f7da3e1f62..1be1ad3d6d 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1225,8 +1225,7 @@ doConnect(void)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) == CONNECTION_BAD)
{
- pg_log_error("connection to database \"%s\" failed: %s",
- dbName, PQerrorMessage(conn));
+ pg_log_error("%s", PQerrorMessage(conn));
PQfinish(conn);
return NULL;
}
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 61b671d54f..daffc18e52 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -90,7 +90,7 @@ pgbench(
1,
[qr{^$}],
[
- qr{connection to database "no-such-database" failed},
+ qr{connection to server .* failed},
qr{FATAL: database "no-such-database" does not exist}
],
'no such database');
diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index 13ac531695..21ef297e6e 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -150,8 +150,7 @@ connectDatabase(const ConnParams *cparams, const char *progname,
PQfinish(conn);
return NULL;
}
- pg_log_error("could not connect to database %s: %s",
- cparams->dbname, PQerrorMessage(conn));
+ pg_log_error("%s", PQerrorMessage(conn));
exit(1);
}
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 1cb52116f9..6b0a3067e6 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -652,7 +652,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
const char *errmsg = PQerrorMessage(this->connection);
const char *db = realname ? realname : ecpg_gettext("<DEFAULT>");
- ecpg_log("ECPGconnect: could not open database: %s\n", errmsg);
+ /* PQerrorMessage's result already has a trailing newline */
+ ecpg_log("ECPGconnect: %s", errmsg);
ecpg_finish(this);
#ifdef ENABLE_THREAD_SAFETY
diff --git a/src/interfaces/ecpg/test/expected/connect-test5.stderr b/src/interfaces/ecpg/test/expected/connect-test5.stderr
index db3cd9c228..a15f344320 100644
--- a/src/interfaces/ecpg/test/expected/connect-test5.stderr
+++ b/src/interfaces/ecpg/test/expected/connect-test5.stderr
@@ -36,8 +36,7 @@
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ECPGconnect: opening database <DEFAULT> on <DEFAULT> port <DEFAULT> for user regress_ecpg_user2
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ECPGconnect: could not open database: connection to server failed: FATAL: database "regress_ecpg_user2" does not exist
-
+[NO_PID]: ECPGconnect: connection to server failed: FATAL: database "regress_ecpg_user2" does not exist
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ecpg_finish: connection main closed
[NO_PID]: sqlca: code: 0, state: 00000
@@ -73,8 +72,7 @@
[NO_PID]: sqlca: code: -220, state: 08003
[NO_PID]: ECPGconnect: opening database <DEFAULT> on <DEFAULT> port <DEFAULT> for user regress_ecpg_user2
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ECPGconnect: could not open database: connection to server failed: FATAL: database "regress_ecpg_user2" does not exist
-
+[NO_PID]: ECPGconnect: connection to server failed: FATAL: database "regress_ecpg_user2" does not exist
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ecpg_finish: connection main closed
[NO_PID]: sqlca: code: 0, state: 00000
diff --git a/src/test/examples/testlibpq.c b/src/test/examples/testlibpq.c
index 18c98083de..0372781eaf 100644
--- a/src/test/examples/testlibpq.c
+++ b/src/test/examples/testlibpq.c
@@ -43,8 +43,7 @@ main(int argc, char **argv)
/* Check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
diff --git a/src/test/examples/testlibpq2.c b/src/test/examples/testlibpq2.c
index 511246763a..6337b315a4 100644
--- a/src/test/examples/testlibpq2.c
+++ b/src/test/examples/testlibpq2.c
@@ -72,8 +72,7 @@ main(int argc, char **argv)
/* Check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
diff --git a/src/test/examples/testlibpq3.c b/src/test/examples/testlibpq3.c
index dda45af859..4f7b791388 100644
--- a/src/test/examples/testlibpq3.c
+++ b/src/test/examples/testlibpq3.c
@@ -138,8 +138,7 @@ main(int argc, char **argv)
/* Check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
diff --git a/src/test/examples/testlibpq4.c b/src/test/examples/testlibpq4.c
index df8e454b5d..dd11bbc46d 100644
--- a/src/test/examples/testlibpq4.c
+++ b/src/test/examples/testlibpq4.c
@@ -29,8 +29,7 @@ check_prepare_conn(PGconn *conn, const char *dbName)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database \"%s\" failed: %s",
- dbName, PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit(1);
}
diff --git a/src/test/examples/testlo.c b/src/test/examples/testlo.c
index fa8da58e1b..6d91681bcf 100644
--- a/src/test/examples/testlo.c
+++ b/src/test/examples/testlo.c
@@ -225,8 +225,7 @@ main(int argc, char **argv)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
diff --git a/src/test/examples/testlo64.c b/src/test/examples/testlo64.c
index 6334171163..23e9109446 100644
--- a/src/test/examples/testlo64.c
+++ b/src/test/examples/testlo64.c
@@ -249,8 +249,7 @@ main(int argc, char **argv)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
- fprintf(stderr, "Connection to database failed: %s",
- PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit_nicely(conn);
}
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index f80261c022..0a73d38dae 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -167,7 +167,7 @@ main(int argc, char **argv)
conns[i] = PQconnectdb(conninfo);
if (PQstatus(conns[i]) != CONNECTION_OK)
{
- fprintf(stderr, "Connection %d to database failed: %s",
+ fprintf(stderr, "Connection %d failed: %s",
i, PQerrorMessage(conns[i]));
exit(1);
}
diff --git a/src/tools/findoidjoins/findoidjoins.c b/src/tools/findoidjoins/findoidjoins.c
index a42c8a34da..f882c8b0ef 100644
--- a/src/tools/findoidjoins/findoidjoins.c
+++ b/src/tools/findoidjoins/findoidjoins.c
@@ -44,7 +44,7 @@ main(int argc, char **argv)
conn = PQconnectdb(sql.data);
if (PQstatus(conn) == CONNECTION_BAD)
{
- fprintf(stderr, "connection error: %s\n", PQerrorMessage(conn));
+ fprintf(stderr, "%s", PQerrorMessage(conn));
exit(EXIT_FAILURE);
}
=
view thread (18+ messages) latest in thread
Message-ID: <1094524.1611266589@sss.pgh.pa.us>
Permalink: ../1094524.1611266589@sss.pgh.pa.us/
Also on: postgresql.org/message-id/1094524.1611266589@sss.pgh.pa.us
copy link · copy postgr.es · copy git am
reply 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-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, robertmhaas@gmail.com, alvherre@alvh.no-ip.org
Subject: Re: strange error reporting
In-Reply-To: <1094524.1611266589@sss.pgh.pa.us>
* 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