mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a couple of contrib/dblink bugs.

Browse Source 
dblink_exec leaked temporary database connections if any error occurred
after connection setup, for example
	SELECT dblink_exec('...connect string...', 'select 1/0');
Add a PG_TRY block to ensure PQfinish gets done when it is needed.
(dblink_record_internal is on the hairy edge of needing similar treatment,
but seems not to be actively broken at the moment.)

Also, in 9.0 and up, only one of the three functions using tuplestore
return mode was properly checking that the query context would allow
a tuplestore result.

Noted while reviewing dblink patch.  Back-patch to all supported branches.
This commit is contained in:
Tom Lane 2012-04-03 20:43:15 -04:00
parent 5e86c61a7e
commit d843ed2116
1 changed files with 116 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

206
contrib/dblink/dblink.c
View File

@ -67,6 +67,7 @@ typedef struct remoteConn
* Internal declarations * Internal declarations
*/ */
static Datum dblink_record_internal(FunctionCallInfo fcinfo, bool is_async); static Datum dblink_record_internal(FunctionCallInfo fcinfo, bool is_async);
static void prepTuplestoreResult(FunctionCallInfo fcinfo);
static void materializeResult(FunctionCallInfo fcinfo, PGresult *res); static void materializeResult(FunctionCallInfo fcinfo, PGresult *res);
static remoteConn *getConnectionByName(const char *name); static remoteConn *getConnectionByName(const char *name);
static HTAB *createConnHash(void); static HTAB *createConnHash(void);
@ -495,7 +496,6 @@ PG_FUNCTION_INFO_V1(dblink_fetch);
Datum Datum
dblink_fetch(PG_FUNCTION_ARGS) dblink_fetch(PG_FUNCTION_ARGS)
{ {
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
PGresult *res = NULL; PGresult *res = NULL;
char *conname = NULL; char *conname = NULL;
remoteConn *rconn = NULL; remoteConn *rconn = NULL;
@ -505,6 +505,8 @@ dblink_fetch(PG_FUNCTION_ARGS)
int howmany = 0; int howmany = 0;
bool fail = true; /* default to backward compatible */ bool fail = true; /* default to backward compatible */
prepTuplestoreResult(fcinfo);
DBLINK_INIT; DBLINK_INIT;
if (PG_NARGS() == 4) if (PG_NARGS() == 4)
@ -551,11 +553,6 @@ dblink_fetch(PG_FUNCTION_ARGS)
if (!conn) if (!conn)
DBLINK_CONN_NOT_AVAIL; DBLINK_CONN_NOT_AVAIL;
/* let the caller know we're sending back a tuplestore */
rsinfo->returnMode = SFRM_Materialize;
rsinfo->setResult = NULL;
rsinfo->setDesc = NULL;
initStringInfo(&buf); initStringInfo(&buf);
appendStringInfo(&buf, "FETCH %d FROM %s", howmany, curname); appendStringInfo(&buf, "FETCH %d FROM %s", howmany, curname);
@ -632,7 +629,6 @@ dblink_get_result(PG_FUNCTION_ARGS)
static Datum static Datum
dblink_record_internal(FunctionCallInfo fcinfo, bool is_async) dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
{ {
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
char *msg; char *msg;
PGresult *res = NULL; PGresult *res = NULL;
PGconn *conn = NULL; PGconn *conn = NULL;
@ -643,16 +639,7 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
bool fail = true; /* default to backward compatible */ bool fail = true; /* default to backward compatible */
bool freeconn = false; bool freeconn = false;
/* check to see if caller supports us returning a tuplestore */ prepTuplestoreResult(fcinfo);
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("set-valued function called in context that cannot accept a set")));
if (!(rsinfo->allowedModes & SFRM_Materialize))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("materialize mode required, but it is not " \
"allowed in this context")));
DBLINK_INIT; DBLINK_INIT;
@ -712,11 +699,6 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
if (!conn) if (!conn)
DBLINK_CONN_NOT_AVAIL; DBLINK_CONN_NOT_AVAIL;
/* let the caller know we're sending back a tuplestore */
rsinfo->returnMode = SFRM_Materialize;
rsinfo->setResult = NULL;
rsinfo->setDesc = NULL;
/* synchronous query, or async result retrieval */ /* synchronous query, or async result retrieval */
if (!is_async) if (!is_async)
res = PQexec(conn, sql); res = PQexec(conn, sql);
@ -745,14 +727,45 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
} }
/* /*
* Materialize the PGresult to return them as the function result. * Verify function caller can handle a tuplestore result, and set up for that.
* The res will be released in this function. *
* Note: if the caller returns without actually creating a tuplestore, the
* executor will treat the function result as an empty set.
*/
static void
prepTuplestoreResult(FunctionCallInfo fcinfo)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
/* check to see if query supports us returning a tuplestore */
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("set-valued function called in context that cannot accept a set")));
if (!(rsinfo->allowedModes & SFRM_Materialize))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("materialize mode required, but it is not allowed in this context")));
/* let the executor know we're sending back a tuplestore */
rsinfo->returnMode = SFRM_Materialize;
/* caller must fill these to return a non-empty result */
rsinfo->setResult = NULL;
rsinfo->setDesc = NULL;
}
/*
* Copy the contents of the PGresult into a tuplestore to be returned
* as the result of the current function.
* The PGresult will be released in this function.
*/ */
static void static void
materializeResult(FunctionCallInfo fcinfo, PGresult *res) materializeResult(FunctionCallInfo fcinfo, PGresult *res)
{ {
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
/* prepTuplestoreResult must have been called previously */
Assert(rsinfo->returnMode == SFRM_Materialize); Assert(rsinfo->returnMode == SFRM_Materialize);
PG_TRY(); PG_TRY();
@ -1004,85 +1017,97 @@ PG_FUNCTION_INFO_V1(dblink_exec);
Datum Datum
dblink_exec(PG_FUNCTION_ARGS) dblink_exec(PG_FUNCTION_ARGS)
{ {
char *msg; text *volatile sql_cmd_status = NULL;
PGresult *res = NULL; PGconn *volatile conn = NULL;
text *sql_cmd_status = NULL; volatile bool freeconn = false;
PGconn *conn = NULL;
char *connstr = NULL;
char *sql = NULL;
char *conname = NULL;
remoteConn *rconn = NULL;
bool freeconn = false;
bool fail = true; /* default to backward compatible behavior */
DBLINK_INIT; DBLINK_INIT;
if (PG_NARGS() == 3) PG_TRY();
{ {
/* must be text,text,bool */ char *msg;
DBLINK_GET_CONN; PGresult *res = NULL;
sql = text_to_cstring(PG_GETARG_TEXT_PP(1)); char *connstr = NULL;
fail = PG_GETARG_BOOL(2); char *sql = NULL;
} char *conname = NULL;
else if (PG_NARGS() == 2) remoteConn *rconn = NULL;
{ bool fail = true; /* default to backward compatible behavior */
/* might be text,text or text,bool */
if (get_fn_expr_argtype(fcinfo->flinfo, 1) == BOOLOID) if (PG_NARGS() == 3)
{ {
/* must be text,text,bool */
DBLINK_GET_CONN;
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
fail = PG_GETARG_BOOL(2);
}
else if (PG_NARGS() == 2)
{
/* might be text,text or text,bool */
if (get_fn_expr_argtype(fcinfo->flinfo, 1) == BOOLOID)
{
conn = pconn->conn;
sql = text_to_cstring(PG_GETARG_TEXT_PP(0));
fail = PG_GETARG_BOOL(1);
}
else
{
DBLINK_GET_CONN;
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
}
}
else if (PG_NARGS() == 1)
{
/* must be single text argument */
conn = pconn->conn; conn = pconn->conn;
sql = text_to_cstring(PG_GETARG_TEXT_PP(0)); sql = text_to_cstring(PG_GETARG_TEXT_PP(0));
fail = PG_GETARG_BOOL(1); }
else
/* shouldn't happen */
elog(ERROR, "wrong number of arguments");
if (!conn)
DBLINK_CONN_NOT_AVAIL;
res = PQexec(conn, sql);
if (!res ||
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
dblink_res_error(conname, res, "could not execute command", fail);
/*
* and save a copy of the command status string to return as our
* result tuple
*/
sql_cmd_status = cstring_to_text("ERROR");
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
/*
* and save a copy of the command status string to return as our
* result tuple
*/
sql_cmd_status = cstring_to_text(PQcmdStatus(res));
PQclear(res);
} }
else else
{ {
DBLINK_GET_CONN; PQclear(res);
sql = text_to_cstring(PG_GETARG_TEXT_PP(1)); ereport(ERROR,
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
errmsg("statement returning results not allowed")));
} }
} }
else if (PG_NARGS() == 1) PG_CATCH();
{ {
/* must be single text argument */ /* if needed, close the connection to the database */
conn = pconn->conn; if (freeconn)
sql = text_to_cstring(PG_GETARG_TEXT_PP(0)); PQfinish(conn);
PG_RE_THROW();
} }
else PG_END_TRY();
/* shouldn't happen */
elog(ERROR, "wrong number of arguments");
if (!conn) /* if needed, close the connection to the database */
DBLINK_CONN_NOT_AVAIL;
res = PQexec(conn, sql);
if (!res ||
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
dblink_res_error(conname, res, "could not execute command", fail);
/*
* and save a copy of the command status string to return as our
* result tuple
*/
sql_cmd_status = cstring_to_text("ERROR");
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
/*
* and save a copy of the command status string to return as our
* result tuple
*/
sql_cmd_status = cstring_to_text(PQcmdStatus(res));
PQclear(res);
}
else
{
PQclear(res);
ereport(ERROR,
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
errmsg("statement returning results not allowed")));
}
/* if needed, close the connection to the database and cleanup */
if (freeconn) if (freeconn)
PQfinish(conn); PQfinish(conn);
@ -1503,13 +1528,15 @@ dblink_get_notify(PG_FUNCTION_ARGS)
MemoryContext per_query_ctx; MemoryContext per_query_ctx;
MemoryContext oldcontext; MemoryContext oldcontext;
prepTuplestoreResult(fcinfo);
DBLINK_INIT; DBLINK_INIT;
if (PG_NARGS() == 1) if (PG_NARGS() == 1)
DBLINK_GET_NAMED_CONN; DBLINK_GET_NAMED_CONN;
else else
conn = pconn->conn; conn = pconn->conn;
/* create the tuplestore */ /* create the tuplestore in per-query memory */
per_query_ctx = rsinfo->econtext->ecxt_per_query_memory; per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
oldcontext = MemoryContextSwitchTo(per_query_ctx); oldcontext = MemoryContextSwitchTo(per_query_ctx);
@ -1522,7 +1549,6 @@ dblink_get_notify(PG_FUNCTION_ARGS)
TEXTOID, -1, 0); TEXTOID, -1, 0);
tupstore = tuplestore_begin_heap(true, false, work_mem); tupstore = tuplestore_begin_heap(true, false, work_mem);
rsinfo->returnMode = SFRM_Materialize;
rsinfo->setResult = tupstore; rsinfo->setResult = tupstore;
rsinfo->setDesc = tupdesc; rsinfo->setDesc = tupdesc;