plpgsql-stmt_call-fix-2.patch
text/x-patch
Filename: plpgsql-stmt_call-fix-2.patch
Type: text/x-patch
Part: 0
Patch
Format: unified
| File | + | − |
|---|---|---|
| src/backend/executor/spi.c | 37 | 4 |
| src/backend/utils/cache/plancache.c | 25 | 1 |
| src/backend/utils/resowner/resowner.c | 8 | 2 |
| src/include/executor/spi_priv.h | 1 | 0 |
| src/include/utils/plancache.h | 3 | 0 |
| src/pl/plpgsql/src/pl_exec.c | 10 | 1 |
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 055ebb77ae..d6bbbc3dbd 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -24,6 +24,7 @@
#include "executor/executor.h"
#include "executor/spi_priv.h"
#include "miscadmin.h"
+#include "storage/proc.h"
#include "tcop/pquery.h"
#include "tcop/utility.h"
#include "utils/builtins.h"
@@ -2223,6 +2224,9 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
CachedPlan *cplan = NULL;
ListCell *lc1;
+ LocalTransactionId before_lxid = MyProc->lxid;
+ bool is_fragile = plan->fragile;
+
/*
* Setup error traceback support for ereport()
*/
@@ -2326,6 +2330,8 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
* plan, the refcount must be backed by the CurrentResourceOwner.
*/
cplan = GetCachedPlan(plansource, paramLI, plan->saved, _SPI_current->queryEnv);
+ cplan->is_fragile = is_fragile;
+
stmt_list = cplan->stmt_list;
/*
@@ -2520,8 +2526,19 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
}
}
- /* Done with this plan, so release refcount */
- ReleaseCachedPlan(cplan, plan->saved);
+ if (is_fragile)
+ {
+ LocalTransactionId after_lxid = MyProc->lxid;;
+
+ if (before_lxid == after_lxid)
+ ReleaseCachedPlan(cplan, plan->saved);
+
+ FinalReleaseFragileCachedPlan(cplan);
+ }
+ else
+ /* Done with this plan, so release refcount */
+ ReleaseCachedPlan(cplan, plan->saved);
+
cplan = NULL;
/*
@@ -2539,9 +2556,25 @@ fail:
if (pushed_active_snap)
PopActiveSnapshot();
- /* We no longer need the cached plan refcount, if any */
if (cplan)
- ReleaseCachedPlan(cplan, plan->saved);
+ {
+ if (is_fragile)
+ {
+ LocalTransactionId after_lxid = MyProc->lxid;;
+
+ /*
+ * Release plan, when the transaction is same. When
+ * there are new transaction, then cached plan was
+ * released by resource owner.
+ */
+ if (before_lxid == after_lxid)
+ ReleaseCachedPlan(cplan, plan->saved);
+
+ FinalReleaseFragileCachedPlan(cplan);
+ }
+ else
+ ReleaseCachedPlan(cplan, plan->saved);
+ }
/*
* Pop the error context stack
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 75b475c179..de3ed036f2 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -528,10 +528,14 @@ ReleaseGenericPlan(CachedPlanSource *plansource)
if (plansource->gplan)
{
CachedPlan *plan = plansource->gplan;
+ bool is_fragile = plan->is_fragile;
Assert(plan->magic == CACHEDPLAN_MAGIC);
plansource->gplan = NULL;
ReleaseCachedPlan(plan, false);
+
+ if (is_fragile)
+ FinalReleaseFragileCachedPlan(plan);
}
}
@@ -1265,9 +1269,12 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwner)
Assert(plan->is_saved);
ResourceOwnerForgetPlanCacheRef(CurrentResourceOwner, plan);
}
+
Assert(plan->refcount > 0);
plan->refcount--;
- if (plan->refcount == 0)
+
+ /* We don't want to destroy fragile plans immediately */
+ if (plan->refcount == 0 && !plan->is_fragile)
{
/* Mark it no longer valid */
plan->magic = 0;
@@ -1278,6 +1285,23 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwner)
}
}
+void
+FinalReleaseFragileCachedPlan(CachedPlan *plan)
+{
+
+ Assert(plan->magic == CACHEDPLAN_MAGIC);
+ Assert(plan->is_fragile);
+
+ if (plan->refcount == 0)
+ {
+ plan->magic = 0;
+
+ /* One-shot plans do not own their context, so we can't free them */
+ if (!plan->is_oneshot)
+ MemoryContextDelete(plan->context);
+ }
+}
+
/*
* CachedPlanAllowsSimpleValidityCheck: can we use CachedPlanIsSimplyValid?
*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 8bc2c4e9ea..b328729ebd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -635,7 +635,13 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
{
CachedPlan *res = (CachedPlan *) DatumGetPointer(foundres);
- if (isCommit)
+ /*
+ * The transaction can be ended inside execution of CALL statements.
+ * In this case, the caller cannot to clean own resources, and then
+ * cleaning by resource owner is expected. Unfortunatelly a caller
+ * routine doesn't know if this situation will be or not.
+ */
+ if (isCommit && !res->is_fragile)
PrintPlanCacheLeakWarning(res);
ReleaseCachedPlan(res, true);
}
@@ -1140,7 +1146,7 @@ ResourceOwnerRememberPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
void
ResourceOwnerForgetPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
{
- if (!ResourceArrayRemove(&(owner->planrefarr), PointerGetDatum(plan)))
+ if (!ResourceArrayRemove(&(owner->planrefarr), PointerGetDatum(plan)) && !plan->is_fragile)
elog(ERROR, "plancache reference %p is not owned by resource owner %s",
plan, owner->name);
}
diff --git a/src/include/executor/spi_priv.h b/src/include/executor/spi_priv.h
index 6220928bd3..e1a01ecc0b 100644
--- a/src/include/executor/spi_priv.h
+++ b/src/include/executor/spi_priv.h
@@ -93,6 +93,7 @@ typedef struct _SPI_plan
bool saved; /* saved or unsaved plan? */
bool oneshot; /* one-shot plan? */
bool no_snapshots; /* let the caller handle the snapshots */
+ bool fragile; /* plancache can be released by resource owner */
List *plancache_list; /* one CachedPlanSource per parsetree */
MemoryContext plancxt; /* Context containing _SPI_plan and data */
int cursor_options; /* Cursor options used for planning */
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 522020d763..d657eb0f17 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -150,6 +150,7 @@ typedef struct CachedPlan
bool is_oneshot; /* is it a "oneshot" plan? */
bool is_saved; /* is CachedPlan in a long-lived context? */
bool is_valid; /* is the stmt_list currently valid? */
+ bool is_fragile; /* cleaning by resource owner is expected */
Oid planRoleId; /* Role ID the plan was created for */
bool dependsOnRole; /* is plan specific to that role? */
TransactionId saved_xmin; /* if valid, replan when TransactionXmin
@@ -222,6 +223,8 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
QueryEnvironment *queryEnv);
extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner);
+extern void FinalReleaseFragileCachedPlan(CachedPlan *plan);
+
extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
CachedPlan *plan,
ResourceOwner owner);
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index d4a3d58daa..0f2ba6b0f6 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2171,7 +2171,7 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
* XXX This would be fixable with some plancache/resowner surgery
* elsewhere, but for now we'll just work around this here.
*/
- exec_prepare_plan(estate, expr, 0, estate->atomic);
+ exec_prepare_plan(estate, expr, 0, true);
/*
* The procedure call could end transactions, which would upset
@@ -2181,6 +2181,15 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
plan = expr->plan;
plan->no_snapshots = true;
+ /*
+ * Plancache have to be released on end of tranaction. But for this
+ * case, transaction can be ended somewhere inside CALL statements
+ * chain. We cannot to know where and why. So we should to mark
+ * this plan as fragile. In this case is possible, there are
+ * more actors are responsible to cleaning plan cache.
+ */
+ plan->fragile = true;
+
/*
* Force target to be recalculated whenever the plan changes, in
* case the procedure's argument list has changed.