Here is a patch to systematically trim the trailing newlines off
PQerrorMessage() results in backend uses (dblink, postgres_fdw,
libpqwalreceiver).
I noticed that there are some inconsistent assumptions about whether
PQerrorMessage() can ever return NULL. From the code, I think that
should not be possible, but some code appears to be prepared for it.
Other code is not. What is correct?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From d6835cff36bcc237462febf5ba7d3df7d560329f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Wed, 8 Feb 2017 09:31:35 -0500
Subject: [PATCH] chomp PQerrorMessage() in backend uses
PQerrorMessage() returns an error message with a trailing newline, but
in backend use (dblink, postgres_fdw, libpqwalreceiver), we want to have
the error message without that for emitting via ereport(). To simplify
that, add a function pchomp() that returns a pstrdup'ed string with the
trailing newline characters removed.
---
contrib/dblink/dblink.c | 16 ++++++------
contrib/dblink/expected/dblink.out | 2 --
contrib/postgres_fdw/connection.c | 14 ++--------
.../libpqwalreceiver/libpqwalreceiver.c | 30 +++++++++++-----------
src/backend/utils/mmgr/mcxt.c | 14 ++++++++++
src/include/utils/palloc.h | 2 ++
6 files changed, 41 insertions(+), 37 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index ac43c458cb..db0a8ba72d 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -166,7 +166,7 @@ typedef struct remoteConnHashEnt
#define DBLINK_RES_INTERNALERROR(p2) \
do { \
- msg = pstrdup(PQerrorMessage(conn)); \
+ msg = pchomp(PQerrorMessage(conn)); \
if (res) \
PQclear(res); \
elog(ERROR, "%s: %s", p2, msg); \
@@ -204,7 +204,7 @@ typedef struct remoteConnHashEnt
conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \
{ \
- msg = pstrdup(PQerrorMessage(conn)); \
+ msg = pchomp(PQerrorMessage(conn)); \
PQfinish(conn); \
ereport(ERROR, \
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), \
@@ -278,7 +278,7 @@ dblink_connect(PG_FUNCTION_ARGS)
if (PQstatus(conn) == CONNECTION_BAD)
{
- msg = pstrdup(PQerrorMessage(conn));
+ msg = pchomp(PQerrorMessage(conn));
PQfinish(conn);
if (rconn)
pfree(rconn);
@@ -651,7 +651,7 @@ dblink_send_query(PG_FUNCTION_ARGS)
/* async query send */
retval = PQsendQuery(conn, sql);
if (retval != 1)
- elog(NOTICE, "could not send query: %s", PQerrorMessage(conn));
+ elog(NOTICE, "could not send query: %s", pchomp(PQerrorMessage(conn)));
PG_RETURN_INT32(retval);
}
@@ -1087,7 +1087,7 @@ storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const char *sql)
PGresult *res;
if (!PQsendQuery(conn, sql))
- elog(ERROR, "could not send query: %s", PQerrorMessage(conn));
+ elog(ERROR, "could not send query: %s", pchomp(PQerrorMessage(conn)));
if (!PQsetSingleRowMode(conn)) /* shouldn't fail */
elog(ERROR, "failed to set single-row mode for dblink query");
@@ -1366,8 +1366,8 @@ dblink_error_message(PG_FUNCTION_ARGS)
DBLINK_INIT;
DBLINK_GET_NAMED_CONN;
- msg = PQerrorMessage(conn);
- if (msg == NULL || msg[0] == '\0')
+ msg = pchomp(PQerrorMessage(conn));
+ if (msg[0] == '\0')
PG_RETURN_TEXT_P(cstring_to_text("OK"));
else
PG_RETURN_TEXT_P(cstring_to_text(msg));
@@ -2709,7 +2709,7 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
* return NULL, not a PGresult at all.
*/
if (message_primary == NULL)
- message_primary = PQerrorMessage(conn);
+ message_primary = pchomp(PQerrorMessage(conn));
if (res)
PQclear(res);
diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
index 5acaaf225a..4b6d26e574 100644
--- a/contrib/dblink/expected/dblink.out
+++ b/contrib/dblink/expected/dblink.out
@@ -377,7 +377,6 @@ FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[])
WHERE t.a > 7;
ERROR: could not establish connection
DETAIL: missing "=" after "myconn" in connection info string
-
-- create a named persistent connection
SELECT dblink_connect('myconn',connection_parameters());
dblink_connect
@@ -604,7 +603,6 @@ FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[])
WHERE t.a > 7;
ERROR: could not establish connection
DETAIL: missing "=" after "myconn" in connection info string
-
-- create a named persistent connection
SELECT dblink_connect('myconn',connection_parameters());
dblink_connect
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 7f7a744ac0..c6e3d44515 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -226,21 +226,11 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
conn = PQconnectdbParams(keywords, values, false);
if (!conn || PQstatus(conn) != CONNECTION_OK)
- {
- char *connmessage;
- int msglen;
-
- /* libpq typically appends a newline, strip that */
- connmessage = pstrdup(PQerrorMessage(conn));
- msglen = strlen(connmessage);
- if (msglen > 0 && connmessage[msglen - 1] == '\n')
- connmessage[msglen - 1] = '\0';
ereport(ERROR,
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
errmsg("could not connect to server \"%s\"",
server->servername),
- errdetail_internal("%s", connmessage)));
- }
+ errdetail_internal("%s", pchomp(PQerrorMessage(conn)))));
/*
* Check that non-superuser has used password to establish connection;
@@ -563,7 +553,7 @@ pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
* return NULL, not a PGresult at all.
*/
if (message_primary == NULL)
- message_primary = PQerrorMessage(conn);
+ message_primary = pchomp(PQerrorMessage(conn));
ereport(elevel,
(errcode(sqlstate),
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 44a89c73fd..b88760fb68 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -141,7 +141,7 @@ libpqrcv_connect(const char *conninfo, bool logical, const char *appname,
conn->streamConn = PQconnectdbParams(keys, vals, /* expand_dbname = */ true);
if (PQstatus(conn->streamConn) != CONNECTION_OK)
{
- *err = pstrdup(PQerrorMessage(conn->streamConn));
+ *err = pchomp(PQerrorMessage(conn->streamConn));
return NULL;
}
@@ -239,7 +239,7 @@ libpqrcv_identify_system(WalReceiverConn *conn, TimeLineID *primary_tli,
ereport(ERROR,
(errmsg("could not receive database system identifier and timeline ID from "
"the primary server: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
}
if (PQnfields(res) < 3 || PQntuples(res) != 1)
{
@@ -316,13 +316,13 @@ libpqrcv_startstreaming(WalReceiverConn *conn,
if (!pubnames_str)
ereport(ERROR,
(errmsg("could not start WAL streaming: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
pubnames_literal = PQescapeLiteral(conn->streamConn, pubnames_str,
strlen(pubnames_str));
if (!pubnames_literal)
ereport(ERROR,
(errmsg("could not start WAL streaming: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
appendStringInfo(&cmd, ", publication_names %s", pubnames_literal);
PQfreemem(pubnames_literal);
pfree(pubnames_str);
@@ -347,7 +347,7 @@ libpqrcv_startstreaming(WalReceiverConn *conn,
PQclear(res);
ereport(ERROR,
(errmsg("could not start WAL streaming: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
}
PQclear(res);
return true;
@@ -366,7 +366,7 @@ libpqrcv_endstreaming(WalReceiverConn *conn, TimeLineID *next_tli)
PQflush(conn->streamConn))
ereport(ERROR,
(errmsg("could not send end-of-streaming message to primary: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
*next_tli = 0;
@@ -410,7 +410,7 @@ libpqrcv_endstreaming(WalReceiverConn *conn, TimeLineID *next_tli)
if (PQresultStatus(res) != PGRES_COMMAND_OK)
ereport(ERROR,
(errmsg("error reading result of streaming command: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
PQclear(res);
/* Verify that there are no more results */
@@ -418,7 +418,7 @@ libpqrcv_endstreaming(WalReceiverConn *conn, TimeLineID *next_tli)
if (res != NULL)
ereport(ERROR,
(errmsg("unexpected result after CommandComplete: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
}
/*
@@ -445,7 +445,7 @@ libpqrcv_readtimelinehistoryfile(WalReceiverConn *conn,
ereport(ERROR,
(errmsg("could not receive timeline history file from "
"the primary server: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
}
if (PQnfields(res) != 2 || PQntuples(res) != 1)
{
@@ -603,7 +603,7 @@ libpqrcv_receive(WalReceiverConn *conn, char **buffer,
if (PQconsumeInput(conn->streamConn) == 0)
ereport(ERROR,
(errmsg("could not receive data from WAL stream: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
/* Now that we've consumed some input, try again */
rawlen = PQgetCopyData(conn->streamConn, &conn->recvBuf, 1);
@@ -630,13 +630,13 @@ libpqrcv_receive(WalReceiverConn *conn, char **buffer,
PQclear(res);
ereport(ERROR,
(errmsg("could not receive data from WAL stream: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
}
}
if (rawlen < -1)
ereport(ERROR,
(errmsg("could not receive data from WAL stream: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
/* Return received messages to caller */
*buffer = conn->recvBuf;
@@ -655,7 +655,7 @@ libpqrcv_send(WalReceiverConn *conn, const char *buffer, int nbytes)
PQflush(conn->streamConn))
ereport(ERROR,
(errmsg("could not send data to WAL stream: %s",
- PQerrorMessage(conn->streamConn))));
+ pchomp(PQerrorMessage(conn->streamConn)))));
}
/*
@@ -689,7 +689,7 @@ libpqrcv_create_slot(WalReceiverConn *conn, const char *slotname,
PQclear(res);
ereport(ERROR,
(errmsg("could not create replication slot \"%s\": %s",
- slotname, PQerrorMessage(conn->streamConn))));
+ slotname, pchomp(PQerrorMessage(conn->streamConn)))));
}
*lsn = DatumGetLSN(DirectFunctionCall1Coll(pg_lsn_in, InvalidOid,
@@ -720,7 +720,7 @@ libpqrcv_command(WalReceiverConn *conn, const char *cmd, char **err)
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
PQclear(res);
- *err = pstrdup(PQerrorMessage(conn->streamConn));
+ *err = pchomp(PQerrorMessage(conn->streamConn));
return false;
}
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 6ad0bb47b6..c21dfcb96b 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1181,3 +1181,17 @@ pnstrdup(const char *in, Size len)
out[len] = '\0';
return out;
}
+
+/*
+ * Make copy of string with all trailing newline characters removed.
+ */
+char *
+pchomp(const char *in)
+{
+ size_t n;
+
+ n = strlen(in);
+ while (n > 0 && in[n - 1] == '\n')
+ n--;
+ return pnstrdup(in, n);
+}
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index b72fe4aee8..2e07bd57ad 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -127,6 +127,8 @@ extern char *MemoryContextStrdup(MemoryContext context, const char *string);
extern char *pstrdup(const char *in);
extern char *pnstrdup(const char *in, Size len);
+extern char *pchomp(const char *in);
+
/* sprintf into a palloc'd buffer --- these are in psprintf.c */
extern char *psprintf(const char *fmt,...) pg_attribute_printf(1, 2);
extern size_t pvsnprintf(char *buf, size_t len, const char *fmt, va_list args) pg_attribute_printf(3, 0);
--
2.11.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
