Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l2i2x-0006yl-Q2 for pgsql-hackers@arkaria.postgresql.org; Thu, 21 Jan 2021 22:03:20 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l2i2w-0006u9-Hj for pgsql-hackers@arkaria.postgresql.org; Thu, 21 Jan 2021 22:03:18 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l2i2w-0006u1-22 for pgsql-hackers@lists.postgresql.org; Thu, 21 Jan 2021 22:03:18 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l2i2s-00042q-2e for pgsql-hackers@postgresql.org; Thu, 21 Jan 2021 22:03:17 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 10LM39Dg1094525; Thu, 21 Jan 2021 17:03:09 -0500 From: Tom Lane To: Robert Haas cc: Alvaro Herrera , "pgsql-hackers@postgresql.org" Subject: Re: strange error reporting In-reply-to: <798280.1611192827@sss.pgh.pa.us> References: <717401.1611163169@sss.pgh.pa.us> <718535.1611164867@sss.pgh.pa.us> <798280.1611192827@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Wed, 20 Jan 2021 20:33:47 -0500" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <1094490.1611266551.0@sss.pgh.pa.us> Date: Thu, 21 Jan 2021 17:03:09 -0500 Message-ID: <1094524.1611266589@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <1094490.1611266551.1@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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="make-connection-failure-messages-less-redundant.patch"; charset="us-ascii" Content-ID: <1094490.1611266551.2@sss.pgh.pa.us> Content-Description: make-connection-failure-messages-less-redundant.patch Content-Transfer-Encoding: quoted-printable 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) =3D=3D 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 *pa= ram) /* check to see that the backend connection was successfully made */ if (PQstatus(conn) =3D=3D 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) !=3D 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) !=3D 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) !=3D 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) !=3D 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) !=3D 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) =3D=3D 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 *conn= ection_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_d= ump.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 se= rver .* 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_nam= e) = if (conn =3D=3D NULL || PQstatus(conn) !=3D 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_nam= e) * get_db_conn() * * get database connection, using named database + standard params for cl= uster + * + * 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 =3D get_db_conn(cluster, "template1")) =3D=3D NULL || PQstatus(conn) !=3D 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 =3D=3D &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) =3D=3D 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/pgbenc= h/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/e= cpglib/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 =3D PQerrorMessage(this->connection); const char *db =3D realname ? realname : ecpg_gettext(""); = - 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 on port for user regress_ecpg_user2 [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: ECPGconnect: could not open database: connection to server fail= ed: FATAL: database "regress_ecpg_user2" does not exist - +[NO_PID]: ECPGconnect: connection to server failed: FATAL: database "reg= ress_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 on port for user regress_ecpg_user2 [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: ECPGconnect: could not open database: connection to server fail= ed: FATAL: database "regress_ecpg_user2" does not exist - +[NO_PID]: ECPGconnect: connection to server failed: FATAL: database "reg= ress_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) !=3D 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) !=3D 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) !=3D 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) !=3D 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) !=3D 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) !=3D 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/iso= lationtester.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] =3D PQconnectdb(conninfo); if (PQstatus(conns[i]) !=3D 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/findoidjoin= s/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 =3D PQconnectdb(sql.data); if (PQstatus(conn) =3D=3D CONNECTION_BAD) { - fprintf(stderr, "connection error: %s\n", PQerrorMessage(conn)); + fprintf(stderr, "%s", PQerrorMessage(conn)); exit(EXIT_FAILURE); } = ------- =_aaaaaaaaaa0--