some dblink refactoring
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
Attachments:
v2-0001-dblink-Replace-some-macros-by-static-functions.patchtext/x-patch; name=v2-0001-dblink-Replace-some-macros-by-static-functions.patchDownload
From 2e1fc0b0c50452bac91461a2317c28a8718fe89f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@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 <mail@joeconway.com>
- * And contributors:
- * Darko Prenosil <Darko.Prenosil@finteh.hr>
- * Shridhar Daithankar <shridhar_daithankar@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
On Tue, Feb 28, 2017 at 10:09 PM, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
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.
+1
Any chance we can make get_connect_string() a core function or at least
externally accessible?
On 2/28/17 22:22, Corey Huinker wrote:
Any chance we can make get_connect_string() a core function or at least
externally accessible?
[get_connect_string() gets the connection string for a foreign server]
The connection string for a foreign server depends on the nature of the
foreign server. dblink can assume it's a PostgreSQL server, but it's
not clear how to generalize that.
Some kind of node or connection registry (i.e., "native" servers) might
be a better feature to think about here.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Peter Eisentraut
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.
I changed the status to ready for committer. The patch applied cleanly, passed the regression test (make installcheck in contrib/dblink/), and the code looks perfect.
How about applying the attached small patch for another refactoring? This merely changes makeStringInfo() to initStringInfo() at two sites just other places in the same file. makeStringInfo() on the function local variables leaves memory for StringInfoData allocated unnecessarily (which may be automatically reclaimed some time after.)
Regards
Takayuki Tsunakawa
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Tsunakawa,
Takayuki
How about applying the attached small patch for another refactoring? This
merely changes makeStringInfo() to initStringInfo() at two sites just other
places in the same file. makeStringInfo() on the function local variables
leaves memory for StringInfoData allocated unnecessarily (which may be
automatically reclaimed some time after.)
Sorry, I forgot to attach the file.
Regards
Takayuki Tsunakawa
Attachments:
dblink_strinfo_refactor.patchapplication/octet-stream; name=dblink_strinfo_refactor.patchDownload
diff -rc a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
*** a/contrib/dblink/dblink.c 2017-03-08 13:44:16.000000000 +0900
--- b/contrib/dblink/dblink.c 2017-03-08 13:57:06.000000000 +0900
***************
*** 2721,2733 ****
ForeignServer *foreign_server = NULL;
UserMapping *user_mapping;
ListCell *cell;
! StringInfo buf = makeStringInfo();
ForeignDataWrapper *fdw;
AclResult aclresult;
char *srvname;
static const PQconninfoOption *options = NULL;
/*
* Get list of valid libpq options.
*
--- 2721,2735 ----
ForeignServer *foreign_server = NULL;
UserMapping *user_mapping;
ListCell *cell;
! StringInfoData buf;
ForeignDataWrapper *fdw;
AclResult aclresult;
char *srvname;
static const PQconninfoOption *options = NULL;
+ initStringInfo(&buf);
+
/*
* Get list of valid libpq options.
*
***************
*** 2769,2775 ****
DefElem *def = lfirst(cell);
if (is_valid_dblink_option(options, def->defname, ForeignDataWrapperRelationId))
! appendStringInfo(buf, "%s='%s' ", def->defname,
escape_param_str(strVal(def->arg)));
}
--- 2771,2777 ----
DefElem *def = lfirst(cell);
if (is_valid_dblink_option(options, def->defname, ForeignDataWrapperRelationId))
! appendStringInfo(&buf, "%s='%s' ", def->defname,
escape_param_str(strVal(def->arg)));
}
***************
*** 2778,2784 ****
DefElem *def = lfirst(cell);
if (is_valid_dblink_option(options, def->defname, ForeignServerRelationId))
! appendStringInfo(buf, "%s='%s' ", def->defname,
escape_param_str(strVal(def->arg)));
}
--- 2780,2786 ----
DefElem *def = lfirst(cell);
if (is_valid_dblink_option(options, def->defname, ForeignServerRelationId))
! appendStringInfo(&buf, "%s='%s' ", def->defname,
escape_param_str(strVal(def->arg)));
}
***************
*** 2788,2798 ****
DefElem *def = lfirst(cell);
if (is_valid_dblink_option(options, def->defname, UserMappingRelationId))
! appendStringInfo(buf, "%s='%s' ", def->defname,
escape_param_str(strVal(def->arg)));
}
! return buf->data;
}
else
return NULL;
--- 2790,2800 ----
DefElem *def = lfirst(cell);
if (is_valid_dblink_option(options, def->defname, UserMappingRelationId))
! appendStringInfo(&buf, "%s='%s' ", def->defname,
escape_param_str(strVal(def->arg)));
}
! return buf.data;
}
else
return NULL;
***************
*** 2807,2822 ****
escape_param_str(const char *str)
{
const char *cp;
! StringInfo buf = makeStringInfo();
for (cp = str; *cp; cp++)
{
if (*cp == '\\' || *cp == '\'')
! appendStringInfoChar(buf, '\\');
! appendStringInfoChar(buf, *cp);
}
! return buf->data;
}
/*
--- 2809,2826 ----
escape_param_str(const char *str)
{
const char *cp;
! StringInfoData buf;
!
! initStringInfo(&buf);
for (cp = str; *cp; cp++)
{
if (*cp == '\\' || *cp == '\'')
! appendStringInfoChar(&buf, '\\');
! appendStringInfoChar(&buf, *cp);
}
! return buf.data;
}
/*
On 3/8/17 00:10, Tsunakawa, Takayuki wrote:
I changed the status to ready for committer. The patch applied cleanly, passed the regression test (make installcheck in contrib/dblink/), and the code looks perfect.
How about applying the attached small patch for another refactoring? This merely changes makeStringInfo() to initStringInfo() at two sites just other places in the same file. makeStringInfo() on the function local variables leaves memory for StringInfoData allocated unnecessarily (which may be automatically reclaimed some time after.)
Committed both. Thanks.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers