Here is a patch to refactor some macro hell in dblink.
This patch was discussed in the background sessions thread as a
prerequisite for some work there, but I figure I'll make a separate
thread for it to give everyone interested in dblink a chance to respond
separate from the other thread.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 2e1fc0b0c50452bac91461a2317c28a8718fe89f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Sun, 25 Dec 2016 12:00:00 -0500
Subject: [PATCH v2 1/2] dblink: Replace some macros by static functions
Also remove some unused code and the no longer useful dblink.h file.
---
contrib/dblink/dblink.c | 290 +++++++++++++++++++++++-------------------------
contrib/dblink/dblink.h | 39 -------
2 files changed, 137 insertions(+), 192 deletions(-)
delete mode 100644 contrib/dblink/dblink.h
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index e0d6778a08..67d6699066 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -61,8 +61,6 @@
#include "utils/tqual.h"
#include "utils/varlena.h"
-#include "dblink.h"
-
PG_MODULE_MAGIC;
typedef struct remoteConn
@@ -146,98 +144,102 @@ typedef struct remoteConnHashEnt
/* initial number of connection hashes */
#define NUMCONN 16
-/* general utility */
-#define xpfree(var_) \
- do { \
- if (var_ != NULL) \
- { \
- pfree(var_); \
- var_ = NULL; \
- } \
- } while (0)
-
-#define xpstrdup(var_c, var_) \
- do { \
- if (var_ != NULL) \
- var_c = pstrdup(var_); \
- else \
- var_c = NULL; \
- } while (0)
-
-#define DBLINK_RES_INTERNALERROR(p2) \
- do { \
- msg = pchomp(PQerrorMessage(conn)); \
- if (res) \
- PQclear(res); \
- elog(ERROR, "%s: %s", p2, msg); \
- } while (0)
-
-#define DBLINK_CONN_NOT_AVAIL \
- do { \
- if(conname) \
- ereport(ERROR, \
- (errcode(ERRCODE_CONNECTION_DOES_NOT_EXIST), \
- errmsg("connection \"%s\" not available", conname))); \
- else \
- ereport(ERROR, \
- (errcode(ERRCODE_CONNECTION_DOES_NOT_EXIST), \
- errmsg("connection not available"))); \
- } while (0)
-
-#define DBLINK_GET_CONN \
- do { \
- char *conname_or_str = text_to_cstring(PG_GETARG_TEXT_PP(0)); \
- rconn = getConnectionByName(conname_or_str); \
- if (rconn) \
- { \
- conn = rconn->conn; \
- conname = conname_or_str; \
- } \
- else \
- { \
- connstr = get_connect_string(conname_or_str); \
- if (connstr == NULL) \
- { \
- connstr = conname_or_str; \
- } \
- dblink_connstr_check(connstr); \
- conn = PQconnectdb(connstr); \
- if (PQstatus(conn) == CONNECTION_BAD) \
- { \
- msg = pchomp(PQerrorMessage(conn)); \
- PQfinish(conn); \
- ereport(ERROR, \
- (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), \
- errmsg("could not establish connection"), \
- errdetail_internal("%s", msg))); \
- } \
- dblink_security_check(conn, rconn); \
- if (PQclientEncoding(conn) != GetDatabaseEncoding()) \
- PQsetClientEncoding(conn, GetDatabaseEncodingName()); \
- freeconn = true; \
- } \
- } while (0)
-
-#define DBLINK_GET_NAMED_CONN \
- do { \
- conname = text_to_cstring(PG_GETARG_TEXT_PP(0)); \
- rconn = getConnectionByName(conname); \
- if (rconn) \
- conn = rconn->conn; \
- else \
- DBLINK_CONN_NOT_AVAIL; \
- } while (0)
-
-#define DBLINK_INIT \
- do { \
- if (!pconn) \
- { \
- pconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext, sizeof(remoteConn)); \
- pconn->conn = NULL; \
- pconn->openCursorCount = 0; \
- pconn->newXactForCursor = FALSE; \
- } \
- } while (0)
+static char *
+xpstrdup(const char *in)
+{
+ if (in == NULL)
+ return NULL;
+ return pstrdup(in);
+}
+
+static void
+dblink_res_internalerror(PGconn *conn, PGresult *res, const char *p2)
+{
+ char *msg = pchomp(PQerrorMessage(conn));
+ if (res)
+ PQclear(res);
+ elog(ERROR, "%s: %s", p2, msg);
+}
+
+static void pg_attribute_noreturn()
+dblink_conn_not_avail(const char *conname)
+{
+ if (conname)
+ ereport(ERROR,
+ (errcode(ERRCODE_CONNECTION_DOES_NOT_EXIST),
+ errmsg("connection \"%s\" not available", conname)));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_CONNECTION_DOES_NOT_EXIST),
+ errmsg("connection not available")));
+}
+
+static void
+dblink_get_conn(char *conname_or_str,
+ PGconn * volatile *conn_p, char **conname_p, volatile bool *freeconn_p)
+{
+ remoteConn *rconn = getConnectionByName(conname_or_str);
+ PGconn *conn;
+ char *conname;
+ bool freeconn;
+
+ if (rconn)
+ {
+ conn = rconn->conn;
+ conname = conname_or_str;
+ freeconn = false;
+ }
+ else
+ {
+ const char *connstr;
+
+ connstr = get_connect_string(conname_or_str);
+ if (connstr == NULL)
+ connstr = conname_or_str;
+ dblink_connstr_check(connstr);
+ conn = PQconnectdb(connstr);
+ if (PQstatus(conn) == CONNECTION_BAD)
+ {
+ char *msg = pchomp(PQerrorMessage(conn));
+ PQfinish(conn);
+ ereport(ERROR,
+ (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
+ errmsg("could not establish connection"),
+ errdetail_internal("%s", msg)));
+ }
+ dblink_security_check(conn, rconn);
+ if (PQclientEncoding(conn) != GetDatabaseEncoding())
+ PQsetClientEncoding(conn, GetDatabaseEncodingName());
+ freeconn = true;
+ conname = NULL;
+ }
+
+ *conn_p = conn;
+ *conname_p = conname;
+ *freeconn_p = freeconn;
+}
+
+static PGconn *
+dblink_get_named_conn(const char *conname)
+{
+ remoteConn *rconn = getConnectionByName(conname);
+ if (rconn)
+ return rconn->conn;
+ else
+ dblink_conn_not_avail(conname);
+}
+
+static void
+dblink_init(void)
+{
+ if (!pconn)
+ {
+ pconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext, sizeof(remoteConn));
+ pconn->conn = NULL;
+ pconn->openCursorCount = 0;
+ pconn->newXactForCursor = FALSE;
+ }
+}
/*
* Create a persistent connection to another database
@@ -253,7 +255,7 @@ dblink_connect(PG_FUNCTION_ARGS)
PGconn *conn = NULL;
remoteConn *rconn = NULL;
- DBLINK_INIT;
+ dblink_init();
if (PG_NARGS() == 2)
{
@@ -318,7 +320,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
remoteConn *rconn = NULL;
PGconn *conn = NULL;
- DBLINK_INIT;
+ dblink_init();
if (PG_NARGS() == 1)
{
@@ -331,7 +333,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
conn = pconn->conn;
if (!conn)
- DBLINK_CONN_NOT_AVAIL;
+ dblink_conn_not_avail(conname);
PQfinish(conn);
if (rconn)
@@ -352,7 +354,6 @@ PG_FUNCTION_INFO_V1(dblink_open);
Datum
dblink_open(PG_FUNCTION_ARGS)
{
- char *msg;
PGresult *res = NULL;
PGconn *conn = NULL;
char *curname = NULL;
@@ -362,7 +363,7 @@ dblink_open(PG_FUNCTION_ARGS)
remoteConn *rconn = NULL;
bool fail = true; /* default to backward compatible behavior */
- DBLINK_INIT;
+ dblink_init();
initStringInfo(&buf);
if (PG_NARGS() == 2)
@@ -401,7 +402,7 @@ dblink_open(PG_FUNCTION_ARGS)
}
if (!rconn || !rconn->conn)
- DBLINK_CONN_NOT_AVAIL;
+ dblink_conn_not_avail(conname);
else
conn = rconn->conn;
@@ -410,7 +411,7 @@ dblink_open(PG_FUNCTION_ARGS)
{
res = PQexec(conn, "BEGIN");
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- DBLINK_RES_INTERNALERROR("begin error");
+ dblink_res_internalerror(conn, res, "begin error");
PQclear(res);
rconn->newXactForCursor = TRUE;
@@ -450,11 +451,10 @@ dblink_close(PG_FUNCTION_ARGS)
char *curname = NULL;
char *conname = NULL;
StringInfoData buf;
- char *msg;
remoteConn *rconn = NULL;
bool fail = true; /* default to backward compatible behavior */
- DBLINK_INIT;
+ dblink_init();
initStringInfo(&buf);
if (PG_NARGS() == 1)
@@ -489,7 +489,7 @@ dblink_close(PG_FUNCTION_ARGS)
}
if (!rconn || !rconn->conn)
- DBLINK_CONN_NOT_AVAIL;
+ dblink_conn_not_avail(conname);
else
conn = rconn->conn;
@@ -517,7 +517,7 @@ dblink_close(PG_FUNCTION_ARGS)
res = PQexec(conn, "COMMIT");
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- DBLINK_RES_INTERNALERROR("commit error");
+ dblink_res_internalerror(conn, res, "commit error");
PQclear(res);
}
}
@@ -543,7 +543,7 @@ dblink_fetch(PG_FUNCTION_ARGS)
prepTuplestoreResult(fcinfo);
- DBLINK_INIT;
+ dblink_init();
if (PG_NARGS() == 4)
{
@@ -587,7 +587,7 @@ dblink_fetch(PG_FUNCTION_ARGS)
}
if (!conn)
- DBLINK_CONN_NOT_AVAIL;
+ dblink_conn_not_avail(conname);
initStringInfo(&buf);
appendStringInfo(&buf, "FETCH %d FROM %s", howmany, curname);
@@ -633,15 +633,13 @@ PG_FUNCTION_INFO_V1(dblink_send_query);
Datum
dblink_send_query(PG_FUNCTION_ARGS)
{
- char *conname = NULL;
- PGconn *conn = NULL;
- char *sql = NULL;
- remoteConn *rconn = NULL;
+ PGconn *conn;
+ char *sql;
int retval;
if (PG_NARGS() == 2)
{
- DBLINK_GET_NAMED_CONN;
+ conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
}
else
@@ -671,15 +669,12 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
prepTuplestoreResult(fcinfo);
- DBLINK_INIT;
+ dblink_init();
PG_TRY();
{
- char *msg;
- char *connstr = NULL;
char *sql = NULL;
char *conname = NULL;
- remoteConn *rconn = NULL;
bool fail = true; /* default to backward compatible */
if (!is_async)
@@ -687,7 +682,7 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
if (PG_NARGS() == 3)
{
/* text,text,bool */
- DBLINK_GET_CONN;
+ dblink_get_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)), &conn, &conname, &freeconn);
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
fail = PG_GETARG_BOOL(2);
}
@@ -702,7 +697,7 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
}
else
{
- DBLINK_GET_CONN;
+ dblink_get_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)), &conn, &conname, &freeconn);
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
}
}
@@ -722,13 +717,13 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
if (PG_NARGS() == 2)
{
/* text,bool */
- DBLINK_GET_NAMED_CONN;
+ conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
fail = PG_GETARG_BOOL(1);
}
else if (PG_NARGS() == 1)
{
/* text */
- DBLINK_GET_NAMED_CONN;
+ conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
}
else
/* shouldn't happen */
@@ -736,7 +731,7 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
}
if (!conn)
- DBLINK_CONN_NOT_AVAIL;
+ dblink_conn_not_avail(conname);
if (!is_async)
{
@@ -1297,12 +1292,10 @@ PG_FUNCTION_INFO_V1(dblink_is_busy);
Datum
dblink_is_busy(PG_FUNCTION_ARGS)
{
- char *conname = NULL;
- PGconn *conn = NULL;
- remoteConn *rconn = NULL;
+ PGconn *conn;
- DBLINK_INIT;
- DBLINK_GET_NAMED_CONN;
+ dblink_init();
+ conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
PQconsumeInput(conn);
PG_RETURN_INT32(PQisBusy(conn));
@@ -1323,15 +1316,13 @@ PG_FUNCTION_INFO_V1(dblink_cancel_query);
Datum
dblink_cancel_query(PG_FUNCTION_ARGS)
{
- int res = 0;
- char *conname = NULL;
- PGconn *conn = NULL;
- remoteConn *rconn = NULL;
+ int res;
+ PGconn *conn;
PGcancel *cancel;
char errbuf[256];
- DBLINK_INIT;
- DBLINK_GET_NAMED_CONN;
+ dblink_init();
+ conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
cancel = PQgetCancel(conn);
res = PQcancel(cancel, errbuf, 256);
@@ -1359,12 +1350,10 @@ Datum
dblink_error_message(PG_FUNCTION_ARGS)
{
char *msg;
- char *conname = NULL;
- PGconn *conn = NULL;
- remoteConn *rconn = NULL;
+ PGconn *conn;
- DBLINK_INIT;
- DBLINK_GET_NAMED_CONN;
+ dblink_init();
+ conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
msg = PQerrorMessage(conn);
if (msg == NULL || msg[0] == '\0')
@@ -1384,22 +1373,19 @@ dblink_exec(PG_FUNCTION_ARGS)
PGconn *volatile conn = NULL;
volatile bool freeconn = false;
- DBLINK_INIT;
+ dblink_init();
PG_TRY();
{
- char *msg;
PGresult *res = NULL;
- char *connstr = NULL;
char *sql = NULL;
char *conname = NULL;
- remoteConn *rconn = NULL;
bool fail = true; /* default to backward compatible behavior */
if (PG_NARGS() == 3)
{
/* must be text,text,bool */
- DBLINK_GET_CONN;
+ dblink_get_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)), &conn, &conname, &freeconn);
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
fail = PG_GETARG_BOOL(2);
}
@@ -1414,7 +1400,7 @@ dblink_exec(PG_FUNCTION_ARGS)
}
else
{
- DBLINK_GET_CONN;
+ dblink_get_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)), &conn, &conname, &freeconn);
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
}
}
@@ -1429,7 +1415,7 @@ dblink_exec(PG_FUNCTION_ARGS)
elog(ERROR, "wrong number of arguments");
if (!conn)
- DBLINK_CONN_NOT_AVAIL;
+ dblink_conn_not_avail(conname);
res = PQexec(conn, sql);
if (!res ||
@@ -1880,9 +1866,7 @@ PG_FUNCTION_INFO_V1(dblink_get_notify);
Datum
dblink_get_notify(PG_FUNCTION_ARGS)
{
- char *conname = NULL;
- PGconn *conn = NULL;
- remoteConn *rconn = NULL;
+ PGconn *conn;
PGnotify *notify;
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
TupleDesc tupdesc;
@@ -1892,9 +1876,9 @@ dblink_get_notify(PG_FUNCTION_ARGS)
prepTuplestoreResult(fcinfo);
- DBLINK_INIT;
+ dblink_init();
if (PG_NARGS() == 1)
- DBLINK_GET_NAMED_CONN;
+ conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
else
conn = pconn->conn;
@@ -2698,10 +2682,10 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
else
sqlstate = ERRCODE_CONNECTION_FAILURE;
- xpstrdup(message_primary, pg_diag_message_primary);
- xpstrdup(message_detail, pg_diag_message_detail);
- xpstrdup(message_hint, pg_diag_message_hint);
- xpstrdup(message_context, pg_diag_context);
+ message_primary = xpstrdup(pg_diag_message_primary);
+ message_detail = xpstrdup(pg_diag_message_detail);
+ message_hint = xpstrdup(pg_diag_message_hint);
+ message_context = xpstrdup(pg_diag_context);
/*
* If we don't get a message from the PGresult, try the PGconn. This
diff --git a/contrib/dblink/dblink.h b/contrib/dblink/dblink.h
deleted file mode 100644
index c96dbe28a8..0000000000
--- a/contrib/dblink/dblink.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * dblink.h
- *
- * Functions returning results from a remote database
- *
- * Joe Conway <m...@joeconway.com>
- * And contributors:
- * Darko Prenosil <darko.preno...@finteh.hr>
- * Shridhar Daithankar <shridhar_daithan...@persistent.co.in>
- *
- * contrib/dblink/dblink.h
- * Copyright (c) 2001-2017, PostgreSQL Global Development Group
- * ALL RIGHTS RESERVED;
- *
- * Permission to use, copy, modify, and distribute this software and its
- * documentation for any purpose, without fee, and without a written agreement
- * is hereby granted, provided that the above copyright notice and this
- * paragraph and the following two paragraphs appear in all copies.
- *
- * IN NO EVENT SHALL THE AUTHOR OR DISTRIBUTORS BE LIABLE TO ANY PARTY FOR
- * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING
- * LOST PROFITS, ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS
- * DOCUMENTATION, EVEN IF THE AUTHOR OR DISTRIBUTORS HAVE BEEN ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- *
- * THE AUTHOR AND DISTRIBUTORS SPECIFICALLY DISCLAIMS ANY WARRANTIES,
- * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
- * AND FITNESS FOR A PARTICULAR PURPOSE. THE SOFTWARE PROVIDED HEREUNDER IS
- * ON AN "AS IS" BASIS, AND THE AUTHOR AND DISTRIBUTORS HAS NO OBLIGATIONS TO
- * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
- *
- */
-
-#ifndef DBLINK_H
-#define DBLINK_H
-
-#include "fmgr.h"
-
-#endif /* DBLINK_H */
--
2.12.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
