From 1cd935609fd47c17f60d8c30b745be936f21f4c3 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 15 Sep 2008 23:37:40 +0000
Subject: [PATCH] Fix caching of foreign-key-checking queries so that when a
replan is needed, we regenerate the SQL query text not merely the plan
derived from it. This is needed to handle contingencies such as renaming of
a table or column used in an FK. Pre-8.3, such cases worked despite the lack
of replanning (because the cached plan needn't actually change), so this is a
regression. Per bug #4417 from Benjamin Bihler.
---
src/backend/executor/spi.c | 32 ++++++++++++++++++++++-
src/backend/utils/adt/ri_triggers.c | 32 ++++++++++++++++++++---
src/backend/utils/cache/plancache.c | 40 ++++++++++++++++++++++++++++-
src/include/executor/spi.h | 3 ++-
src/include/utils/plancache.h | 3 ++-
5 files changed, 103 insertions(+), 7 deletions(-)
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 443ee57e682..302109f24f2 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.197 2008/07/18 20:26:06 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.198 2008/09/15 23:37:39 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1367,6 +1367,36 @@ SPI_is_cursor_plan(SPIPlanPtr plan)
return false;
}
+/*
+ * SPI_plan_is_valid --- test whether a SPI plan is currently valid
+ * (that is, not marked as being in need of revalidation).
+ *
+ * See notes for CachedPlanIsValid before using this.
+ */
+bool
+SPI_plan_is_valid(SPIPlanPtr plan)
+{
+ Assert(plan->magic == _SPI_PLAN_MAGIC);
+ if (plan->saved)
+ {
+ ListCell *lc;
+
+ foreach(lc, plan->plancache_list)
+ {
+ CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc);
+
+ if (!CachedPlanIsValid(plansource))
+ return false;
+ }
+ return true;
+ }
+ else
+ {
+ /* An unsaved plan is assumed valid for its (short) lifetime */
+ return true;
+ }
+}
+
/*
* SPI_result_code_string --- convert any SPI return code to a string
*
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 7f564e09fa0..8fe29ea260b 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -15,7 +15,7 @@
*
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.109 2008/05/19 04:14:24 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.110 2008/09/15 23:37:39 tgl Exp $
*
* ----------
*/
@@ -3615,6 +3615,7 @@ static SPIPlanPtr
ri_FetchPreparedPlan(RI_QueryKey *key)
{
RI_QueryHashEntry *entry;
+ SPIPlanPtr plan;
/*
* On the first call initialize the hashtable
@@ -3630,7 +3631,30 @@ ri_FetchPreparedPlan(RI_QueryKey *key)
HASH_FIND, NULL);
if (entry == NULL)
return NULL;
- return entry->plan;
+
+ /*
+ * Check whether the plan is still valid. If it isn't, we don't want
+ * to simply rely on plancache.c to regenerate it; rather we should
+ * start from scratch and rebuild the query text too. This is to cover
+ * cases such as table/column renames. We depend on the plancache
+ * machinery to detect possible invalidations, though.
+ *
+ * CAUTION: this check is only trustworthy if the caller has already
+ * locked both FK and PK rels.
+ */
+ plan = entry->plan;
+ if (plan && SPI_plan_is_valid(plan))
+ return plan;
+
+ /*
+ * Otherwise we might as well flush the cached plan now, to free a
+ * little memory space before we make a new one.
+ */
+ entry->plan = NULL;
+ if (plan)
+ SPI_freeplan(plan);
+
+ return NULL;
}
@@ -3653,11 +3677,13 @@ ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan)
ri_InitHashTables();
/*
- * Add the new plan.
+ * Add the new plan. We might be overwriting an entry previously
+ * found invalid by ri_FetchPreparedPlan.
*/
entry = (RI_QueryHashEntry *) hash_search(ri_query_cache,
(void *) key,
HASH_ENTER, &found);
+ Assert(!found || entry->plan == NULL);
entry->plan = plan;
}
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index bf1adf16ecc..48addf3f539 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -35,7 +35,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.21 2008/09/09 18:58:08 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.22 2008/09/15 23:37:39 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -571,6 +571,44 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwner)
MemoryContextDelete(plan->context);
}
+/*
+ * CachedPlanIsValid: test whether the plan within a CachedPlanSource is
+ * currently valid (that is, not marked as being in need of revalidation).
+ *
+ * This result is only trustworthy (ie, free from race conditions) if
+ * the caller has acquired locks on all the relations used in the plan.
+ */
+bool
+CachedPlanIsValid(CachedPlanSource *plansource)
+{
+ CachedPlan *plan;
+
+ /* Validity check that we were given a CachedPlanSource */
+ Assert(list_member_ptr(cached_plans_list, plansource));
+
+ plan = plansource->plan;
+ if (plan && !plan->dead)
+ {
+ /*
+ * Plan must have positive refcount because it is referenced by
+ * plansource; so no need to fear it disappears under us here.
+ */
+ Assert(plan->refcount > 0);
+
+ /*
+ * Although we don't want to acquire locks here, it still seems
+ * useful to check for expiration of a transient plan.
+ */
+ if (TransactionIdIsValid(plan->saved_xmin) &&
+ !TransactionIdEquals(plan->saved_xmin, TransactionXmin))
+ plan->dead = true;
+ else
+ return true;
+ }
+
+ return false;
+}
+
/*
* AcquireExecutorLocks: acquire locks needed for execution of a fully-planned
* cached plan; or release them if acquire is false.
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index 9d3f65b8cf0..82e9757f9d1 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -6,7 +6,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.66 2008/04/01 03:09:30 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.67 2008/09/15 23:37:40 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -118,6 +118,7 @@ extern int SPI_freeplan(SPIPlanPtr plan);
extern Oid SPI_getargtypeid(SPIPlanPtr plan, int argIndex);
extern int SPI_getargcount(SPIPlanPtr plan);
extern bool SPI_is_cursor_plan(SPIPlanPtr plan);
+extern bool SPI_plan_is_valid(SPIPlanPtr plan);
extern const char *SPI_result_code_string(int code);
extern HeapTuple SPI_copytuple(HeapTuple tuple);
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index e19da85e93a..e84a3e742bd 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -8,7 +8,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/utils/plancache.h,v 1.13 2008/09/09 18:58:09 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/plancache.h,v 1.14 2008/09/15 23:37:40 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -109,6 +109,7 @@ extern void DropCachedPlan(CachedPlanSource *plansource);
extern CachedPlan *RevalidateCachedPlan(CachedPlanSource *plansource,
bool useResOwner);
extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner);
+extern bool CachedPlanIsValid(CachedPlanSource *plansource);
extern TupleDesc PlanCacheComputeResultDesc(List *stmt_list);
extern void ResetPlanCache(void);
--
GitLab