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.

5 files changed, 103 insertions, 7 deletions

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index f79868630f6..0b404228d80 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.188.2.1 2008/04/02 18:32:00 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.188.2.2 2008/09/15 23:37:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1230,6 +1230,36 @@ SPI_is_cursor_plan(SPIPlanPtr plan)
 }
 
 /*
+ * 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
  *
  * This is often useful in error messages.	Most callers will probably
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index acb1118f649..874d6464a31 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.103.2.2 2008/05/19 04:14:33 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.103.2.3 2008/09/15 23:37:49 tgl Exp $
  *
  * ----------
  */
@@ -3610,6 +3610,7 @@ static SPIPlanPtr
 ri_FetchPreparedPlan(RI_QueryKey *key)
 {
 	RI_QueryHashEntry *entry;
+	SPIPlanPtr		plan;
 
 	/*
 	 * On the first call initialize the hashtable
@@ -3625,7 +3626,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;
 }
 
 
@@ -3648,11 +3672,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 b66c112926e..bbfdd888c1c 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -33,7 +33,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.15 2008/01/01 19:45:53 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.15.2.1 2008/09/15 23:37:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -602,6 +602,44 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwner)
 }
 
 /*
+ * 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 e64fc39cba5..32d9488a667 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.65 2008/01/01 19:45:57 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.65.2.1 2008/09/15 23:37:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -114,6 +114,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 f0e2c901317..cd734ba4066 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.11 2008/01/01 19:45:59 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/utils/plancache.h,v 1.11.2.1 2008/09/15 23:37:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -107,6 +107,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);