]> granicus.if.org Git - postgresql/commitdiff
Fix caching of foreign-key-checking queries so that when a replan is needed,
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 15 Sep 2008 23:37:40 +0000 (23:37 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 15 Sep 2008 23:37:40 +0000 (23:37 +0000)
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
src/backend/utils/adt/ri_triggers.c
src/backend/utils/cache/plancache.c
src/include/executor/spi.h
src/include/utils/plancache.h

index 443ee57e682929e8b8d72b62a546a2682a6bacf2..302109f24f2fbd72fea72b628f9ea9d6b8c44e8f 100644 (file)
@@ -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
  *
index 7f564e09fa0e1aa062aa4a7352d62c9d6bc8ce57..8fe29ea260bb004cd5030942bb2bc24fec3683c7 100644 (file)
@@ -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;
 }
 
index bf1adf16eccbead06896091fb18d15cec6fa5cc2..48addf3f53939412aefec6a059213381f2bf76a1 100644 (file)
@@ -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.
index 9d3f65b8cf09826f6fe4b787614a1a6c6d0f9cff..82e9757f9d113374f2accd6508f07208dfcde097 100644 (file)
@@ -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);
index e19da85e93ac7013ff7788008edf1ecd694c3623..e84a3e742bd0c23b622bcf59c043899c34a35630 100644 (file)
@@ -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);