I wrote:
> Or we could add fields recording the current transaction/subtransaction
> IDs, then throw away and regenerate the function cache entry if that
> subxact is no longer active. This would be a bit more invasive/riskier
> than throwing a "not supported" error, but it would preserve the
> functionality.
The attached patch seems fairly reasonable for this.
regards, tom lane
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 9a9724574bec2874710316fa74643ae24d2d305e..e62286f9f98eccfd9a30e2e8f4908e8757b48672 100644
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** GetCurrentSubTransactionId(void)
*** 570,575 ****
--- 570,596 ----
return s->subTransactionId;
}
+ /*
+ * SubTransactionIsActive
+ *
+ * Test if the specified subxact ID is still active. Note caller is
+ * responsible for checking whether this ID is relevant to the current xact.
+ */
+ bool
+ SubTransactionIsActive(SubTransactionId subxid)
+ {
+ TransactionState s;
+
+ for (s = CurrentTransactionState; s != NULL; s = s->parent)
+ {
+ if (s->state == TRANS_ABORT)
+ continue;
+ if (s->subTransactionId == subxid)
+ return true;
+ }
+ return false;
+ }
+
/*
* GetCurrentCommandId
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 2782b55e876078cda87bc9dac6b980340be0298d..c908f34cfe4bdd7fb0e059679d8307b2f75c8b23 100644
*** a/src/backend/executor/functions.c
--- b/src/backend/executor/functions.c
***************
*** 25,34 ****
--- 25,36 ----
#include "nodes/nodeFuncs.h"
#include "parser/parse_coerce.h"
#include "parser/parse_func.h"
+ #include "storage/proc.h"
#include "tcop/utility.h"
#include "utils/builtins.h"
#include "utils/datum.h"
#include "utils/lsyscache.h"
+ #include "utils/memutils.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
*************** typedef struct execution_state
*** 74,81 ****
* and linked to from the fn_extra field of the FmgrInfo struct.
*
* Note that currently this has only the lifespan of the calling query.
! * Someday we might want to consider caching the parse/plan results longer
! * than that.
*/
typedef struct
{
--- 76,92 ----
* and linked to from the fn_extra field of the FmgrInfo struct.
*
* Note that currently this has only the lifespan of the calling query.
! * Someday we should rewrite this code to use plancache.c to save parse/plan
! * results for longer than that.
! *
! * Physically, though, the data has the lifespan of the FmgrInfo that's used
! * to call the function, and there are cases (particularly with indexes)
! * where the FmgrInfo might survive across transactions. We cannot assume
! * that the parse/plan trees are good for longer than the (sub)transaction in
! * which parsing was done, so we must mark the record with the LXID/subxid of
! * its creation time, and regenerate everything if that's obsolete. To avoid
! * memory leakage when we do have to regenerate things, all the data is kept
! * in a sub-context of the FmgrInfo's fn_mcxt.
*/
typedef struct
{
*************** typedef struct
*** 106,111 ****
--- 117,128 ----
* track of where the original query boundaries are.
*/
List *func_state;
+
+ MemoryContext fcontext; /* memory context holding this struct and all
+ * subsidiary data */
+
+ LocalTransactionId lxid; /* lxid in which cache was made */
+ SubTransactionId subxid; /* subxid in which cache was made */
} SQLFunctionCache;
typedef SQLFunctionCache *SQLFunctionCachePtr;
*************** static void
*** 551,556 ****
--- 568,575 ----
init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
{
Oid foid = finfo->fn_oid;
+ MemoryContext fcontext;
+ MemoryContext oldcontext;
Oid rettype;
HeapTuple procedureTuple;
Form_pg_proc procedureStruct;
*************** init_sql_fcache(FmgrInfo *finfo, Oid col
*** 562,568 ****
--- 581,605 ----
Datum tmp;
bool isNull;
+ /*
+ * Create memory context that holds all the SQLFunctionCache data. It
+ * must be a child of whatever context holds the FmgrInfo.
+ */
+ fcontext = AllocSetContextCreate(finfo->fn_mcxt,
+ "SQL function data",
+ ALLOCSET_DEFAULT_MINSIZE,
+ ALLOCSET_DEFAULT_INITSIZE,
+ ALLOCSET_DEFAULT_MAXSIZE);
+
+ oldcontext = MemoryContextSwitchTo(fcontext);
+
+ /*
+ * Create the struct proper, link it to fcontext and fn_extra. Once this
+ * is done, we'll be able to recover the memory after failure, even if the
+ * FmgrInfo is long-lived.
+ */
fcache = (SQLFunctionCachePtr) palloc0(sizeof(SQLFunctionCache));
+ fcache->fcontext = fcontext;
finfo->fn_extra = (void *) fcache;
/*
*************** init_sql_fcache(FmgrInfo *finfo, Oid col
*** 635,640 ****
--- 672,682 ----
* sublist, for example if the last query rewrites to DO INSTEAD NOTHING.
* (It might not be unreasonable to throw an error in such a case, but
* this is the historical behavior and it doesn't seem worth changing.)
+ *
+ * Note: since parsing and planning is done in fcontext, we will generate
+ * a lot of cruft that lives as long as the fcache does. This is annoying
+ * but we'll not worry about it until the module is rewritten to use
+ * plancache.c.
*/
raw_parsetree_list = pg_parse_query(fcache->src);
*************** init_sql_fcache(FmgrInfo *finfo, Oid col
*** 700,706 ****
--- 742,754 ----
fcache,
lazyEvalOK);
+ /* Mark fcache with time of creation to show it's valid */
+ fcache->lxid = MyProc->lxid;
+ fcache->subxid = GetCurrentSubTransactionId();
+
ReleaseSysCache(procedureTuple);
+
+ MemoryContextSwitchTo(oldcontext);
}
/* Start up execution of one execution_state node */
*************** postquel_get_single_result(TupleTableSlo
*** 923,931 ****
Datum
fmgr_sql(PG_FUNCTION_ARGS)
{
- MemoryContext oldcontext;
SQLFunctionCachePtr fcache;
ErrorContextCallback sqlerrcontext;
bool randomAccess;
bool lazyEvalOK;
bool is_first;
--- 971,979 ----
Datum
fmgr_sql(PG_FUNCTION_ARGS)
{
SQLFunctionCachePtr fcache;
ErrorContextCallback sqlerrcontext;
+ MemoryContext oldcontext;
bool randomAccess;
bool lazyEvalOK;
bool is_first;
*************** fmgr_sql(PG_FUNCTION_ARGS)
*** 937,949 ****
ListCell *eslc;
/*
- * Switch to context in which the fcache lives. This ensures that
- * parsetrees, plans, etc, will have sufficient lifetime. The
- * sub-executor is responsible for deleting per-tuple information.
- */
- oldcontext = MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt);
-
- /*
* Setup error traceback support for ereport()
*/
sqlerrcontext.callback = sql_exec_error_callback;
--- 985,990 ----
*************** fmgr_sql(PG_FUNCTION_ARGS)
*** 978,997 ****
}
/*
! * Initialize fcache (build plans) if first time through.
*/
fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra;
if (fcache == NULL)
{
init_sql_fcache(fcinfo->flinfo, PG_GET_COLLATION(), lazyEvalOK);
fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra;
}
! eslist = fcache->func_state;
/*
* Find first unfinished query in function, and note whether it's the
* first query.
*/
es = NULL;
is_first = true;
foreach(eslc, eslist)
--- 1019,1061 ----
}
/*
! * Initialize fcache (build plans) if first time through; or re-initialize
! * if the cache is stale.
*/
fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra;
+
+ if (fcache != NULL)
+ {
+ if (fcache->lxid != MyProc->lxid ||
+ !SubTransactionIsActive(fcache->subxid))
+ {
+ /* It's stale; unlink and delete */
+ fcinfo->flinfo->fn_extra = NULL;
+ MemoryContextDelete(fcache->fcontext);
+ fcache = NULL;
+ }
+ }
+
if (fcache == NULL)
{
init_sql_fcache(fcinfo->flinfo, PG_GET_COLLATION(), lazyEvalOK);
fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra;
}
!
! /*
! * Switch to context in which the fcache lives. This ensures that our
! * tuplestore etc will have sufficient lifetime. The sub-executor is
! * responsible for deleting per-tuple information. (XXX in the case of a
! * long-lived FmgrInfo, this policy represents more memory leakage, but
! * it's not entirely clear where to keep stuff instead.)
! */
! oldcontext = MemoryContextSwitchTo(fcache->fcontext);
/*
* Find first unfinished query in function, and note whether it's the
* first query.
*/
+ eslist = fcache->func_state;
es = NULL;
is_first = true;
foreach(eslc, eslist)
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 09e6a6842c2039dc2b7c99cad74a6e83a6d9e9e6..835f6acbee0e10ee51e5a2295429efd5141e3b77 100644
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** extern TransactionId GetCurrentTransacti
*** 215,220 ****
--- 215,221 ----
extern TransactionId GetCurrentTransactionIdIfAny(void);
extern TransactionId GetStableLatestTransactionId(void);
extern SubTransactionId GetCurrentSubTransactionId(void);
+ extern bool SubTransactionIsActive(SubTransactionId subxid);
extern CommandId GetCurrentCommandId(bool used);
extern TimestampTz GetCurrentTransactionStartTimestamp(void);
extern TimestampTz GetCurrentStatementStartTimestamp(void);
--
Sent via pgsql-bugs mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
