Re: strange error reporting - Mailing list pgsql-hackers

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

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"
doesnot 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"
doesnot 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"
doesnot 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);
     }