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 |
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 |
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 |
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 |
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 |
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 |
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 |
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. 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) |
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 |
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 |
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 |
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); } |
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) |
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 |
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) |
Free forum by Nabble | Edit this page |