strange error reporting

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

strange error reporting

Robert Haas
I just made the mistake of trying to run pgbench without first running
createdb and got this:

pgbench: error: connection to database "" failed: could not connect to
socket "/tmp/.s.PGSQL.5432": FATAL:  database "rhaas" does not exist

This looks pretty bogus because (1) I was not attempting to connect to
a database whose name is the empty string and (2) saying that it
couldn't connect to the socket is wrong, else it would not also be
showing a server message.

I haven't investigated why this is happening; apologies if this is a
known issue.

--
Robert Haas
EDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Tom Lane-2
Robert Haas <[hidden email]> writes:
> I just made the mistake of trying to run pgbench without first running
> createdb and got this:

> pgbench: error: connection to database "" failed: could not connect to
> socket "/tmp/.s.PGSQL.5432": FATAL:  database "rhaas" does not exist

> This looks pretty bogus because (1) I was not attempting to connect to
> a database whose name is the empty string and (2) saying that it
> couldn't connect to the socket is wrong, else it would not also be
> showing a server message.

I'm not sure about the empty DB name in the first part (presumably
that's from pgbench, so what was your pgbench command exactly?).
But the 'could not connect to socket' part is a consequence of my
recent fiddling with libpq's connection failure reporting, see
52a10224e.  We could discuss exactly how that ought to be spelled,
but the idea is to consistently identify the host that we were trying
to connect to.  If you have a multi-host connection string, it's
conceivable that "rhaas" exists on some of those hosts and not others,
so I do not think the info is irrelevant.

Just looking at this, I wonder if we ought to drop pgbench's
contribution to the message entirely; it seems like libpq's
message is now fairly freestanding.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Robert Haas
On Wed, Jan 20, 2021 at 12:19 PM Tom Lane <[hidden email]> wrote:

> Robert Haas <[hidden email]> writes:
> > I just made the mistake of trying to run pgbench without first running
> > createdb and got this:
>
> > pgbench: error: connection to database "" failed: could not connect to
> > socket "/tmp/.s.PGSQL.5432": FATAL:  database "rhaas" does not exist
>
> > This looks pretty bogus because (1) I was not attempting to connect to
> > a database whose name is the empty string and (2) saying that it
> > couldn't connect to the socket is wrong, else it would not also be
> > showing a server message.
>
> I'm not sure about the empty DB name in the first part (presumably
> that's from pgbench, so what was your pgbench command exactly?).

I think it was just 'pgbench -i 40'. For sure, I didn't specify a database name.

> But the 'could not connect to socket' part is a consequence of my
> recent fiddling with libpq's connection failure reporting, see
> 52a10224e.  We could discuss exactly how that ought to be spelled,
> but the idea is to consistently identify the host that we were trying
> to connect to.  If you have a multi-host connection string, it's
> conceivable that "rhaas" exists on some of those hosts and not others,
> so I do not think the info is irrelevant.

I'm not saying that which socket I used is totally irrelevant although
in most cases it's going to be a lot of detail. I'm just saying that,
at least for me, when you say you can't connect to a socket, I at
least think about the return value of connect(2), which was clearly 0
here.

> Just looking at this, I wonder if we ought to drop pgbench's
> contribution to the message entirely; it seems like libpq's
> message is now fairly freestanding.

Maybe it would be better if it said:

connection to database at socket "/tmp/.s.PGSQL.5432" failed: FATAL:
database "rhaas" does not exist

--
Robert Haas
EDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Tom Lane-2
Robert Haas <[hidden email]> writes:
> On Wed, Jan 20, 2021 at 12:19 PM Tom Lane <[hidden email]> wrote:
>> But the 'could not connect to socket' part is a consequence of my
>> recent fiddling with libpq's connection failure reporting, see
>> 52a10224e.  We could discuss exactly how that ought to be spelled,
>> but the idea is to consistently identify the host that we were trying
>> to connect to.  If you have a multi-host connection string, it's
>> conceivable that "rhaas" exists on some of those hosts and not others,
>> so I do not think the info is irrelevant.

> I'm not saying that which socket I used is totally irrelevant although
> in most cases it's going to be a lot of detail. I'm just saying that,
> at least for me, when you say you can't connect to a socket, I at
> least think about the return value of connect(2), which was clearly 0
> here.

Fair.  One possibility, which'd take a few more cycles in libpq but
likely not anything significant, is to replace "could not connect to ..."
with "while connecting to ..." once we're past the connect() per se.

> Maybe it would be better if it said:

> connection to database at socket "/tmp/.s.PGSQL.5432" failed: FATAL:
> database "rhaas" does not exist

I'd be inclined to spell it "connection to server at ... failed",
but that sort of wording is surely also possible.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Robert Haas
On Wed, Jan 20, 2021 at 12:47 PM Tom Lane <[hidden email]> wrote:
> Fair.  One possibility, which'd take a few more cycles in libpq but
> likely not anything significant, is to replace "could not connect to ..."
> with "while connecting to ..." once we're past the connect() per se.

Yeah. I think this is kind of a client-side version of errcontext(),
except we don't really have that context formally, so we're trying to
figure out how to fake it in specific cases.

> > Maybe it would be better if it said:
>
> > connection to database at socket "/tmp/.s.PGSQL.5432" failed: FATAL:
> > database "rhaas" does not exist
>
> I'd be inclined to spell it "connection to server at ... failed",
> but that sort of wording is surely also possible.

"connection to server" rather than "connection to database" works for
me; in fact, I think I like it slightly better.

--
Robert Haas
EDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Álvaro Herrera
In reply to this post by Robert Haas
On 2021-Jan-20, Robert Haas wrote:

> On Wed, Jan 20, 2021 at 12:19 PM Tom Lane <[hidden email]> wrote:
> > Robert Haas <[hidden email]> writes:
> > > I just made the mistake of trying to run pgbench without first running
> > > createdb and got this:
> >
> > > pgbench: error: connection to database "" failed: could not connect to
> > > socket "/tmp/.s.PGSQL.5432": FATAL:  database "rhaas" does not exist
> >
> > > This looks pretty bogus because (1) I was not attempting to connect to
> > > a database whose name is the empty string [...]
> >
> > I'm not sure about the empty DB name in the first part (presumably
> > that's from pgbench, so what was your pgbench command exactly?).
>
> I think it was just 'pgbench -i 40'. For sure, I didn't specify a database name.

That's because pgbench reports the input argument dbname, but since you
didn't specify anything, then PQconnectdbParams() uses the libpq
behavior.  I think we'd have to use PQdb() instead.

--
Álvaro Herrera       Valdivia, Chile


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Robert Haas
On Wed, Jan 20, 2021 at 1:25 PM Alvaro Herrera <[hidden email]> wrote:
> That's because pgbench reports the input argument dbname, but since you
> didn't specify anything, then PQconnectdbParams() uses the libpq
> behavior.  I think we'd have to use PQdb() instead.

I figured it was something like that. I don't know whether the right
thing is to use something like PQdb() to get the correct database
name, or whether we should go with Tom's suggestion and omit that
detail altogether, but I think showing the empty string when the user
relied on the default is too confusing.

--
Robert Haas
EDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Álvaro Herrera
On 2021-Jan-20, Robert Haas wrote:

> On Wed, Jan 20, 2021 at 1:25 PM Alvaro Herrera <[hidden email]> wrote:
> > That's because pgbench reports the input argument dbname, but since you
> > didn't specify anything, then PQconnectdbParams() uses the libpq
> > behavior.  I think we'd have to use PQdb() instead.
>
> I figured it was something like that. I don't know whether the right
> thing is to use something like PQdb() to get the correct database
> name, or whether we should go with Tom's suggestion and omit that
> detail altogether, but I think showing the empty string when the user
> relied on the default is too confusing.
Well, the patch seems small enough, and I don't think it'll be in any
way helpful to omit that detail.

--
Álvaro Herrera                            39°49'30"S 73°17'W
"Having your biases confirmed independently is how scientific progress is
made, and hence made our great society what it is today" (Mary Gardiner)

pgbench-db.patch (441 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Tom Lane-2
Alvaro Herrera <[hidden email]> writes:
> On 2021-Jan-20, Robert Haas wrote:
>> I figured it was something like that. I don't know whether the right
>> thing is to use something like PQdb() to get the correct database
>> name, or whether we should go with Tom's suggestion and omit that
>> detail altogether, but I think showing the empty string when the user
>> relied on the default is too confusing.

> Well, the patch seems small enough, and I don't think it'll be in any
> way helpful to omit that detail.

I'm +1 for applying and back-patching that.  I still think we might
want to just drop the phrase altogether in HEAD, but we wouldn't do
that in the back branches, and the message is surely misleading as-is.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Robert Haas
On Wed, Jan 20, 2021 at 1:54 PM Tom Lane <[hidden email]> wrote:

> Alvaro Herrera <[hidden email]> writes:
> > On 2021-Jan-20, Robert Haas wrote:
> >> I figured it was something like that. I don't know whether the right
> >> thing is to use something like PQdb() to get the correct database
> >> name, or whether we should go with Tom's suggestion and omit that
> >> detail altogether, but I think showing the empty string when the user
> >> relied on the default is too confusing.
>
> > Well, the patch seems small enough, and I don't think it'll be in any
> > way helpful to omit that detail.
>
> I'm +1 for applying and back-patching that.  I still think we might
> want to just drop the phrase altogether in HEAD, but we wouldn't do
> that in the back branches, and the message is surely misleading as-is.

Sure, that makes sense.

--
Robert Haas
EDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Tom Lane-2
In reply to this post by Robert Haas
Robert Haas <[hidden email]> writes:
>>> Maybe it would be better if it said:
>>> connection to database at socket "/tmp/.s.PGSQL.5432" failed: FATAL:
>>> database "rhaas" does not exist

>> I'd be inclined to spell it "connection to server at ... failed",
>> but that sort of wording is surely also possible.

> "connection to server" rather than "connection to database" works for
> me; in fact, I think I like it slightly better.

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: "

(or maybe "server on socket"?  "at" sounds right for the IP address
case, but it feels a little off in the socket pathname case.)

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Tom Lane-2
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" 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);
  }
 
Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Álvaro Herrera
In reply to this post by Robert Haas
On 2021-Jan-20, Robert Haas wrote:

> On Wed, Jan 20, 2021 at 1:54 PM Tom Lane <[hidden email]> wrote:
> > Alvaro Herrera <[hidden email]> writes:

> > > Well, the patch seems small enough, and I don't think it'll be in any
> > > way helpful to omit that detail.
> >
> > I'm +1 for applying and back-patching that.  I still think we might
> > want to just drop the phrase altogether in HEAD, but we wouldn't do
> > that in the back branches, and the message is surely misleading as-is.
>
> Sure, that makes sense.

OK, I pushed it.  Thanks,

pgbench has one occurrence of the old pattern in master, in line 6043.
However, since doConnect() returns NULL when it gets CONNECTION_BAD,
that seems dead code.  This patch kills it.

--
Álvaro Herrera                            39°49'30"S 73°17'W
"I can see support will not be a problem.  10 out of 10."    (Simon Wittber)
      (http://archives.postgresql.org/pgsql-general/2004-12/msg00159.php)

deadcode.patch (629 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Tom Lane-2
Alvaro Herrera <[hidden email]> writes:
> pgbench has one occurrence of the old pattern in master, in line 6043.
> However, since doConnect() returns NULL when it gets CONNECTION_BAD,
> that seems dead code.  This patch kills it.

Oh ... I missed that because it wasn't adjacent to the PQconnectdbParams
call :-(.  You're right, that's dead code and we should just delete it.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: strange error reporting

Álvaro Herrera
On 2021-Jan-26, Tom Lane wrote:

> Alvaro Herrera <[hidden email]> writes:
> > pgbench has one occurrence of the old pattern in master, in line 6043.
> > However, since doConnect() returns NULL when it gets CONNECTION_BAD,
> > that seems dead code.  This patch kills it.
>
> Oh ... I missed that because it wasn't adjacent to the PQconnectdbParams
> call :-(.  You're right, that's dead code and we should just delete it.

Pushed, thanks.

--
Álvaro Herrera                            39°49'30"S 73°17'W
"Pensar que el espectro que vemos es ilusorio no lo despoja de espanto,
sólo le suma el nuevo terror de la locura" (Perelandra, C.S.Lewis)