8000 Fix a couple of contrib/dblink bugs. · jandas/postgres@49281db · GitHub
[go: up one dir, main page]
Skip to content

Commit 49281db

Browse files
tglsfdctglsfdc
committed
Fix a couple of contrib/dblink bugs.
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.
1 parent e1a6679 commit 49281db

File tree

1 file changed

+115
-89
lines changed

1 file changed

+115
-89
lines changed

‎contrib/dblink/dblink.c

Lines changed: 115 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ typedef struct remoteConn
8181
* Internal declarations
8282
*/
8383
static Datum dblink_record_internal(FunctionCallInfo fcinfo, bool is_async);
84+
static void prepTuplestoreResult(FunctionCallInfo fcinfo);
8485
sta 8000 tic void materializeResult(FunctionCallInfo fcinfo, PGresult *res);
8586
static remoteConn *getConnectionByName(const char *name);
8687
static HTAB *createConnHash(void);
@@ -510,7 +511,6 @@ PG_FUNCTION_INFO_V1(dblink_fetch);
510511
Datum
511512
dblink_fetch(PG_FUNCTION_ARGS)
512513
{
513-
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
514514
PGresult *res = NULL;
515515
char *conname = NULL;
516516
remoteConn *rconn = NULL;
@@ -520,6 +520,8 @@ dblink_fetch(PG_FUNCTION_ARGS)
520520
int howmany = 0;
521521
bool fail = true; /* default to backward compatible */
522522

523+
prepTuplestoreResult(fcinfo);
524+
523525
DBLINK_INIT;
524526

525527
if (PG_NARGS() == 4)
@@ -566,11 +568,6 @@ dblink_fetch(PG_FUNCTION_ARGS)
566568
if (!conn)
567569
DBLINK_CONN_NOT_AVAIL;
568570

569-
/* let the caller know we're sending back a tuplestore */
570-
rsinfo->returnMode = SFRM_Materialize;
571-
rsinfo->setResult = NULL;
572-
rsinfo->setDesc = NULL;
573-
574571
initStringInfo(&buf);
575572
appendStringInfo(&buf, "FETCH %d FROM %s", howmany, curname);
576573

@@ -650,7 +647,6 @@ dblink_get_result(PG_FUNCTION_ARGS)
650647
static Datum
651648
dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
652649
{
653-
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
654650
char *msg;
655651
PGresult *res = NULL;
656652
PGconn *conn = NULL;
@@ -661,16 +657,7 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
661657
bool fail = true; /* default to backward compatible */
662658
bool freeconn = false;
663659

664-
/* check to see if caller supports us r 8000 eturning a tuplestore */
665-
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
666-
ereport(ERROR,
667-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
668-
errmsg("set-valued function called in context that cannot accept a set")));
669-
if (!(rsinfo->allowedModes & SFRM_Materialize))
670-
ereport(ERROR,
671-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
672-
errmsg("materialize mode required, but it is not " \
673-
"allowed in this context")));
660+
prepTuplestoreResult(fcinfo);
674661

675662
DBLINK_INIT;
676663

@@ -730,11 +717,6 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
730717
if (!conn)
731718
DBLINK_CONN_NOT_AVAIL;
732719

733-
/* let the caller know we're sending back a tuplestore */
734-
rsinfo->returnMode = SFRM_Materialize;
735-
rsinfo->setResult = NULL;
736-
rsinfo->setDesc = NULL;
737-
738720
/* synchronous query, or async result retrieval */
739721
if (!is_async)
740722
res = PQexec(conn, sql);
@@ -763,14 +745,45 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
763745
}
764746

765747
/*
766-
* Materialize the PGresult to return them as the function result.
767-
* The res will be released in this function.
748+
* Verify function caller can handle a tuplestore result, and set up for that.
749+
*
750+
* Note: if the caller returns without actually creating a tuplestore, the
751+
* executor will treat the function result as an empty set.
752+
*/
753+
static void
754+
prepTuplestoreResult(FunctionCallInfo fcinfo)
755+
{
756+
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
757+
758+
/* check to see if query supports us returning a tuplestore */
759+
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
760+
ereport(ERROR,
761+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
762+
errmsg("set-valued function called in context that cannot accept a set")));
763+
if (!(rsinfo->allowedModes & SFRM_Materialize))
764+
ereport(ERROR,
765+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
766+
errmsg("materialize mode required, but it is not allowed in this context")));
767+
768+
/* let the executor know we're sending back a tuplestore */
769+
rsinfo->returnMode = SFRM_Materialize;
770+
771+
/* caller must fill these to return a non-empty result */
772+
rsinfo->setResult = NULL;
773+
rsinfo->setDesc = NULL;
774+
}
775+
776+
/*
777+
* Copy the contents of the PGresult into a tuplestore to be returned
778+
* as the result of the current function.
779+
* The PGresult will be released in this function.
768780
*/
769781
static void
770782
materializeResult(FunctionCallInfo fcinfo, PGresult *res)
771783
{
772784
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
773785

786+
/* prepTuplestoreResult must have been called previously */
774787
Assert(rsinfo->returnMode == SFRM_Materialize);
775788

776789
PG_TRY();
@@ -1022,85 +1035,97 @@ PG_FUNCTION_INFO_V1(dblink_exec);
10221035
Datum
10231036
dblink_exec(PG_FUNCTION_ARGS)
10241037
{
1025-
char *msg;
1026-
PGresult *res = NULL;
1027-
text *sql_cmd_status = NULL;
1028-
PGconn *conn = NULL;
1029-
char *connstr = NULL;
1030-
char *sql = NULL;
1031-
char *conname = NULL;
1032-
remoteConn *rconn = NULL;
1033-
bool freeconn = false;
1034-
bool fail = true; /* default to backward compatible behavior */
1038+
text *volatile sql_cmd_status = NULL;
1039+
PGconn *volatile conn = NULL;
1040+
volatile bool freeconn = false;
10351041

10361042
DBLINK_INIT;
10371043

1038-
if (PG_NARGS() == 3)
1039-
{
1040-
/* must be text,text,bool */
1041-
DBLINK_GET_CONN;
1042-
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
1043-
fail = PG_GETARG_BOOL(2);
1044-
}
1045-
else if (PG_NARGS() == 2)
1044+
PG_TRY();
10461045
{
1047-
/* might be text,text or text,bool */
1048-
if (get_fn_expr_argtype(fcinfo->flinfo, 1) == BOOLOID)
1046+
char *msg;
1047+
PGresult *res = NULL;
1048+
char *connstr = NULL;
1049+
char *sql = NULL;
1050+
char *conname = NULL;
1051+
remoteConn *rconn = NULL;
1052+
bool fail = true; /* default to backward compatible behavior */
1053+
1054+
if (PG_NARGS() == 3)
10491055
{
1056+
/* must be text,text,bool */
1057+
DBLINK_GET_CONN;
1058+
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
1059+
fail = PG_GETARG_BOOL(2);
1060+
}
1061+
else if (PG_NARGS() == 2)
1062+
{
1063+
/* might be text,text or text,bool */
1064+
if (get_fn_expr_argtype(fcinfo->flinfo, 1) == BOOLOID)
1065+
{
1066+
conn = pconn->conn;
1067+
sql = text_to_cstring(PG_GETARG_TEXT_PP(0));
1068+
fail = PG_GETARG_BOOL(1);
1069+
}
1070+
else
1071+
{
1072+
DBLINK_GET_CONN;
1073+
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
1074+
}
1075+
}
1076+
else if (PG_NARGS() == 1)
1077+
{
1078+
/* must be single text argument */
10501079
conn = pconn->conn;
10511080
sql = text_to_cstring(PG_GETARG_TEXT_PP(0));
1052-
fail = PG_GETARG_BOOL(1);
10531081
}
10541082
else
1055-
{
1056-
DBLINK_GET_CONN;
1057-
sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
1058-
}
1059-
}
1060-
else if (PG_NARGS() == 1)
1061-
{
1062-
/* must be single text argument */
1063-
conn = pconn->conn;
1064-
sql = text_to_cstring(PG_GETARG_TEXT_PP(0));
1065-
}
1066-
else
1067-
/* shouldn't happen */
1068-
elog(ERROR, "wrong number of arguments");
1083+
/* shouldn't happen */
1084+
elog(ERROR, "wrong number of arguments");
10691085

1070-
if (!conn)
1071-
DBLINK_CONN_NOT_AVAIL;
1086+
if (!conn)
1087+
DBLINK_CONN_NOT_AVAIL;
10721088

1073-
res = PQexec(conn, sql);
1074-
if (!res ||
1075-
(PQresultStatus(res) != PGRES_COMMAND_OK &&
1076-
PQresultStatus(res) != PGRES_TUPLES_OK))
1077-
{
1078-
dblink_res_error(conname, res, "could not execute command", fail);
1089+
res = PQexec(conn, sql);
1090+
if (!res ||
1091+
(PQresultStatus(res) != PGRES_COMMAND_OK &&
1092+
PQresultStatus(res) != PGRES_TUPLES_OK))
1093+
{
1094+
dblink_res_error(conname, res, "could not execute command", fail);
10791095

1080-
/*
1081-
* and save a copy of the command status string to return as our
1082-
* result tuple
1083-
*/
1084-
sql_cmd_status = cstring_to_text("ERROR");
1085-
}
1086-
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
1087-
{
1088-
/*
1089-
* and save a copy of the command status string to return as our
1090-
* result tuple
1091-
*/
1092-
sql_cmd_status = cstring_to_text(PQcmdStatus(res));
1093-
PQclear(res);
1096+
/*
1097+
* and save a copy of the command status string to return as our
1098+
* result tuple
1099+
*/
1100+
sql_cmd_status = cstring_to_text("ERROR");
1101+
}
1102+
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
1103+
{
1104+
/*
1105+
* and save a copy of the command status string to return as our
1106+
* result tuple
1107+
*/
1108+
sql_cmd_status = cstring_to_text(PQcmdStatus(res));
1109+
PQclear(res);
1110+
}
1111+
else
1112+
{
1113+
PQclear(res);
1114+
ereport(ERROR,
1115+
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
1116+
errmsg("statement returning results not allowed")));
1117+
}
10941118
}
1095-
else
1119+
PG_CATCH();
10961120
{
1097-
PQclear(res);
1098-
ereport(ERROR,
1099-
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
1100-
errmsg("statement returning results not allowed")));
1121+
/* if needed, close the connection to the database */
1122+
if (freeconn)
1123+
PQfinish(conn);
1124+
PG_RE_THROW();
11011125
}
1126+
PG_END_TRY();
11021127

1103-
/* if needed, close the connection to the database and cleanup */
1128+
/* if needed, close the connection to the database */
11041129
if (freeconn)
11051130
PQfinish(conn);
11061131

@@ -1521,13 +1546,15 @@ dblink_get_notify(PG_FUNCTION_ARGS)
15211546
MemoryContext per_query_ctx;
15221547
MemoryContext oldcontext;
15231548

1549+
prepTuplestoreResult(fcinfo);
1550+
15241551
DBLINK_INIT;
15251552
if (PG_NARGS() == 1)
15261553
DBLINK_GET_NAMED_CONN;
15271554
else
15281555
conn = pconn->conn;
15291556

1530-
/* create the tuplestore */
1557+
/* create the tuplestore in per-query memory */
15311558
per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
15321559
oldcontext = MemoryContextSwitchTo(per_query_ctx);
15331560

@@ -1540,7 +1567,6 @@ dblink_get_notify(PG_FUNCTION_ARGS)
15401567
TEXTOID, -1, 0);
15411568

15421569
tupstore = tuplestore_begin_heap(true, false, work_mem);
1543-
rsinfo->returnMode = SFRM_Materialize;
15441570
rsinfo->setResult = tupstore;
15451571
rsinfo->setDesc = tupdesc;
15461572

0 commit comments

Comments
 (0)
0