Change plan caching to honor, not resist, changes in search_path.

In the initial implementation of plan caching, we saved the active
search_path when a plan was first cached, then reinstalled that path
anytime we needed to reparse or replan.  The idea of that was to try to
reselect the same referenced objects, in somewhat the same way that views
continue to refer to the same objects in the face of schema or name
changes.  Of course, that analogy doesn't bear close inspection, since
holding the search_path fixed doesn't cope with object drops or renames.
Moreover sticking with the old path seems to create more surprises than
it avoids.  So instead of doing that, consider that the cached plan depends
on search_path, and force reparse/replan if the active search_path is
different than it was when we last saved the plan.

This gets us fairly close to having "transparency" of plan caching, in the
sense that the cached statement acts the same as if you'd just resubmitted
the original query text for another execution.  There are still some corner
cases where this fails though: a new object added in the search path
schema(s) might capture a reference in the query text, but we'd not realize
that and force a reparse.  We might try to fix that in the future, but for
the moment it looks too expensive and complicated.

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 95cf4b6b467aa96fe8d09ed6816be984d0ad3eb6..b20916353137d9ba577349b42e43170748f80648 100644 (file)
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -4258,7 +4258,9 @@ $$ LANGUAGE plpgsql;
     on specific parameter values, and caching that for re-use.  Typically
     this will happen only if the execution plan is not very sensitive to
     the values of the PL/pgSQL variables referenced in it.
-    If it is, generating a plan each time is a net win.
+    If it is, generating a plan each time is a net win.  See 
+    linkend="sql-prepare"> for more information about the behavior of
+    prepared statements.
    
 
    


diff --git a/doc/src/sgml/ref/prepare.sgml b/doc/src/sgml/ref/prepare.sgml

index 8466a63c58060eaf0d15b9767caa693e037681d8..b1698f2bb8827f13e1bd56e95b585b483d3539db 100644 (file)

--- a/doc/src/sgml/ref/prepare.sgml
+++ b/doc/src/sgml/ref/prepare.sgml
@@ -152,6 +152,28 @@ PREPARE name [ ( 
    documentation.
   
 
+  
+   Although the main point of a prepared statement is to avoid repeated parse
+   analysis and planning of the statement, PostgreSQL will
+   force re-analysis and re-planning of the statement before using it
+   whenever database objects used in the statement have undergone
+   definitional (DDL) changes since the previous use of the prepared
+   statement.  Also, if the value of  changes
+   from one use to the next, the statement will be re-parsed using the new
+   search_path.  (This latter behavior is new as of
+   PostgreSQL 9.3.)  These rules make use of a
+   prepared statement semantically almost equivalent to re-submitting the
+   same query text over and over, but with a performance benefit if no object
+   definitions are changed, especially if the best plan remains the same
+   across uses.  An example of a case where the semantic equivalence is not
+   perfect is that if the statement refers to a table by an unqualified name,
+   and then a new table of the same name is created in a schema appearing
+   earlier in the search_path, no automatic re-parse will occur
+   since no object used in the statement changed.  However, if some other
+   change forces a re-parse, the new table will be referenced in subsequent
+   uses.
+  
+
   
    You can see all prepared statements available in the session by querying the
    pg_prepared_statements


diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml

index 13391689c7010de3c930d8a85823c8fd6fa46290..68693667b66caafe420d1ea4a1286d8db2a85205 100644 (file)

--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -975,6 +975,20 @@ SPIPlanPtr SPI_prepare(const char * command, int 
    plans respectively.
   
 
+  
+   Although the main point of a prepared statement is to avoid repeated parse
+   analysis and planning of the statement, PostgreSQL will
+   force re-analysis and re-planning of the statement before using it
+   whenever database objects used in the statement have undergone
+   definitional (DDL) changes since the previous use of the prepared
+   statement.  Also, if the value of  changes
+   from one use to the next, the statement will be re-parsed using the new
+   search_path.  (This latter behavior is new as of
+   PostgreSQL 9.3.)  See 
+   linkend="sql-prepare"> for more information about the behavior of prepared
+   statements.
+  
+
   
    This function should only be called from a connected procedure.
   


diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c

index b256498d4511add3f55c35f07fecf6a92f19e763..ca4635dc51f41b050521d65cd601bccab90b6726 100644 (file)

--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3096,6 +3096,28 @@ CopyOverrideSearchPath(OverrideSearchPath *path)
    return result;
 }
 
+/*
+ * OverrideSearchPathMatchesCurrent - does path match current setting?
+ */
+bool
+OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
+{
+   /* Easiest way to do this is GetOverrideSearchPath() and compare */
+   bool        result;
+   OverrideSearchPath *cur;
+
+   cur = GetOverrideSearchPath(CurrentMemoryContext);
+   if (path->addCatalog == cur->addCatalog &&
+       path->addTemp == cur->addTemp &&
+       equal(path->schemas, cur->schemas))
+       result = true;
+   else
+       result = false;
+   list_free(cur->schemas);
+   pfree(cur);
+   return result;
+}
+
 /*
  * PushOverrideSearchPath - temporarily override the search path
  *


diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c

index cbc7c498d0d533b149fe4395992d9910a2d6b5cd..4630c44e7407c98a2de2df7bafc2753e78bfcfc6 100644 (file)

--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -15,13 +15,15 @@
  * that matches the event is marked invalid, as is its generic CachedPlan
  * if it has one.  When (and if) the next demand for a cached plan occurs,
  * parse analysis and rewrite is repeated to build a new valid query tree,
- * and then planning is performed as normal.
+ * and then planning is performed as normal.  We also force re-analysis and
+ * re-planning if the active search_path is different from the previous time.
  *
  * Note that if the sinval was a result of user DDL actions, parse analysis
  * could throw an error, for example if a column referenced by the query is
- * no longer present.  The creator of a cached plan can specify whether it
- * is allowable for the query to change output tupdesc on replan (this
- * could happen with "SELECT *" for example) --- if so, it's up to the
+ * no longer present.  Another possibility is for the query's output tupdesc
+ * to change (for instance "SELECT *" might expand differently than before).
+ * The creator of a cached plan can specify whether it is allowable for the
+ * query to change output tupdesc on replan --- if so, it's up to the
  * caller to notice changes and cope with them.
  *
  * Currently, we track exactly the dependencies of plans on relations and
@@ -174,11 +176,11 @@ CreateCachedPlan(Node *raw_parse_tree,
    plansource->cursor_options = 0;
    plansource->fixed_result = false;
    plansource->resultDesc = NULL;
-   plansource->search_path = NULL;
    plansource->context = source_context;
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
    plansource->query_context = NULL;
    plansource->gplan = NULL;
    plansource->is_oneshot = false;
@@ -239,11 +241,11 @@ CreateOneShotCachedPlan(Node *raw_parse_tree,
    plansource->cursor_options = 0;
    plansource->fixed_result = false;
    plansource->resultDesc = NULL;
-   plansource->search_path = NULL;
    plansource->context = CurrentMemoryContext;
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
    plansource->query_context = NULL;
    plansource->gplan = NULL;
    plansource->is_oneshot = true;
@@ -360,6 +362,14 @@ CompleteCachedPlan(CachedPlanSource *plansource,
                                   &plansource->relationOids,
                                   &plansource->invalItems);
 
+   /*
+    * Also save the current search_path in the query_context.  (This should
+    * not generate much extra cruft either, since almost certainly the path
+    * is already valid.)  Again, don't really need it for one-shot plans.
+    */
+   if (!plansource->is_oneshot)
+       plansource->search_path = GetOverrideSearchPath(querytree_context);
+
    /*
     * Save the final parameter types (or other parameter specification data)
     * into the source_context, as well as our other parameters.  Also save
@@ -383,12 +393,6 @@ CompleteCachedPlan(CachedPlanSource *plansource,
 
    MemoryContextSwitchTo(oldcxt);
 
-   /*
-    * Fetch current search_path into dedicated context, but do any
-    * recalculation work required in caller's context.
-    */
-   plansource->search_path = GetOverrideSearchPath(source_context);
-
    plansource->is_complete = true;
    plansource->is_valid = true;
 }
@@ -546,6 +550,23 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
        return NIL;
    }
 
+   /*
+    * If the query is currently valid, we should have a saved search_path ---
+    * check to see if that matches the current environment.  If not, we want
+    * to force replan.
+    */
+   if (plansource->is_valid)
+   {
+       Assert(plansource->search_path != NULL);
+       if (!OverrideSearchPathMatchesCurrent(plansource->search_path))
+       {
+           /* Invalidate the querytree and generic plan */
+           plansource->is_valid = false;
+           if (plansource->gplan)
+               plansource->gplan->is_valid = false;
+       }
+   }
+
    /*
     * If the query is currently valid, acquire locks on the referenced
     * objects; then check again.  We need to do it this way to cover the race
@@ -578,6 +599,7 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
 
    /*
     * Free the query_context.  We don't really expect MemoryContextDelete to
@@ -602,14 +624,6 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
     */
    Assert(plansource->is_complete);
 
-   /*
-    * Restore the search_path that was in use when the plan was made. See
-    * comments for PushOverrideSearchPath about limitations of this.
-    *
-    * (XXX is there anything else we really need to restore?)
-    */
-   PushOverrideSearchPath(plansource->search_path);
-
    /*
     * If a snapshot is already set (the normal case), we can just use that
     * for parsing/planning.  But if it isn't, install one.  Note: no point in
@@ -645,9 +659,6 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
    if (snapshot_set)
        PopActiveSnapshot();
 
-   /* Now we can restore current search path */
-   PopOverrideSearchPath();
-
    /*
     * Check or update the result tupdesc.  XXX should we use a weaker
     * condition than equalTupleDescs() here?
@@ -699,6 +710,13 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
                               &plansource->relationOids,
                               &plansource->invalItems);
 
+   /*
+    * Also save the current search_path in the query_context.  (This should
+    * not generate much extra cruft either, since almost certainly the path
+    * is already valid.)
+    */
+   plansource->search_path = GetOverrideSearchPath(querytree_context);
+
    MemoryContextSwitchTo(oldcxt);
 
    /* Now reparent the finished query_context and save the links */
@@ -848,20 +866,6 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
            qlist = plansource->query_list;
    }
 
-   /*
-    * Restore the search_path that was in use when the plan was made. See
-    * comments for PushOverrideSearchPath about limitations of this.
-    *
-    * (XXX is there anything else we really need to restore?)
-    *
-    * Note: it's a bit annoying to do this and snapshot-setting twice in the
-    * case where we have to do both re-analysis and re-planning.  However,
-    * until there's some evidence that the cost is actually meaningful
-    * compared to parse analysis + planning, I'm not going to contort the
-    * code enough to avoid that.
-    */
-   PushOverrideSearchPath(plansource->search_path);
-
    /*
     * If a snapshot is already set (the normal case), we can just use that
     * for planning.  But if it isn't, and we need one, install one.
@@ -894,9 +898,6 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
    if (snapshot_set)
        PopActiveSnapshot();
 
-   /* Now we can restore current search path */
-   PopOverrideSearchPath();
-
    /*
     * Normally we make a dedicated memory context for the CachedPlan and its
     * subsidiary data.  (It's probably not going to be large, but just in
@@ -1268,7 +1269,6 @@ CopyCachedPlan(CachedPlanSource *plansource)
        newsource->resultDesc = CreateTupleDescCopy(plansource->resultDesc);
    else
        newsource->resultDesc = NULL;
-   newsource->search_path = CopyOverrideSearchPath(plansource->search_path);
    newsource->context = source_context;
 
    querytree_context = AllocSetContextCreate(source_context,
@@ -1280,6 +1280,8 @@ CopyCachedPlan(CachedPlanSource *plansource)
    newsource->query_list = (List *) copyObject(plansource->query_list);
    newsource->relationOids = (List *) copyObject(plansource->relationOids);
    newsource->invalItems = (List *) copyObject(plansource->invalItems);
+   if (plansource->search_path)
+       newsource->search_path = CopyOverrideSearchPath(plansource->search_path);
    newsource->query_context = querytree_context;
 
    newsource->gplan = NULL;


diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h

index af82072edb7e90f243b0976337ae7d68fd2b9ce8..c37df8686ea2e835145d1de4b9ba358014b3a4f2 100644 (file)

--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -128,6 +128,7 @@ extern void ResetTempTableNamespace(void);
 
 extern OverrideSearchPath *GetOverrideSearchPath(MemoryContext context);
 extern OverrideSearchPath *CopyOverrideSearchPath(OverrideSearchPath *path);
+extern bool OverrideSearchPathMatchesCurrent(OverrideSearchPath *path);
 extern void PushOverrideSearchPath(OverrideSearchPath *newpath);
 extern void PopOverrideSearchPath(void);
 


diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h

index 8185427fc4b28ca3caca33f1d177439d626f708e..abaf9dc59c763b4b39978a49b52d5c85c19ed732 100644 (file)

--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -86,12 +86,13 @@ typedef struct CachedPlanSource
    int         cursor_options; /* cursor options used for planning */
    bool        fixed_result;   /* disallow change in result tupdesc? */
    TupleDesc   resultDesc;     /* result type; NULL = doesn't return tuples */
-   struct OverrideSearchPath *search_path;     /* saved search_path */
    MemoryContext context;      /* memory context holding all above */
    /* These fields describe the current analyzed-and-rewritten query tree: */
    List       *query_list;     /* list of Query nodes, or NIL if not valid */
    List       *relationOids;   /* OIDs of relations the queries depend on */
    List       *invalItems;     /* other dependencies, as PlanInvalItems */
+   struct OverrideSearchPath *search_path;     /* search_path used for
+                                                * parsing and planning */
    MemoryContext query_context;    /* context holding the above, or NULL */
    /* If we have a generic plan, this is a reference-counted link to it: */
    struct CachedPlan *gplan;   /* generic plan, or NULL if not valid */


diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out

index f0aa102334be6dce39c58ecd0135792e0ea46c90..864f70f7b54e7dbb021296f5213f023d53201dd0 100644 (file)

--- a/src/test/regress/expected/plancache.out
+++ b/src/test/regress/expected/plancache.out
@@ -163,7 +163,7 @@ select cache_test_2();
         10007
 (1 row)
 
---- Check that change of search_path is ignored by replans
+--- Check that change of search_path is honored when re-using cached plan
 create schema s1
   create table abc (f1 int);
 create schema s2
@@ -188,14 +188,14 @@ select f1 from abc;
 execute p1;
  f1  
 -----
- 123
+ 456
 (1 row)
 
 alter table s1.abc add column f2 float8;   -- force replan
 execute p1;
  f1  
 -----
- 123
+ 456
 (1 row)
 
 drop schema s1 cascade;


diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql

index 26848168f0631c1f423d32305e2a7d4d273428b0..bc2086166b985d7a3886d2899862443edee928bd 100644 (file)

--- a/src/test/regress/sql/plancache.sql
+++ b/src/test/regress/sql/plancache.sql
@@ -94,7 +94,7 @@ create or replace temp view v1 as
   select 2+2+4+(select max(unique1) from tenk1) as f1;
 select cache_test_2();
 
---- Check that change of search_path is ignored by replans
+--- Check that change of search_path is honored when re-using cached plan
 
 create schema s1
   create table abc (f1 int);

diff --git a/doc/src/sgml/ref/prepare.sgml b/doc/src/sgml/ref/prepare.sgml
index 8466a63c58060eaf0d15b9767caa693e037681d8..b1698f2bb8827f13e1bd56e95b585b483d3539db 100644 (file)
--- a/doc/src/sgml/ref/prepare.sgml
+++ b/doc/src/sgml/ref/prepare.sgml
@@ -152,6 +152,28 @@ PREPARE name [ ( 
    documentation.
   
 
+  
+   Although the main point of a prepared statement is to avoid repeated parse
+   analysis and planning of the statement, PostgreSQL will
+   force re-analysis and re-planning of the statement before using it
+   whenever database objects used in the statement have undergone
+   definitional (DDL) changes since the previous use of the prepared
+   statement.  Also, if the value of  changes
+   from one use to the next, the statement will be re-parsed using the new
+   search_path.  (This latter behavior is new as of
+   PostgreSQL 9.3.)  These rules make use of a
+   prepared statement semantically almost equivalent to re-submitting the
+   same query text over and over, but with a performance benefit if no object
+   definitions are changed, especially if the best plan remains the same
+   across uses.  An example of a case where the semantic equivalence is not
+   perfect is that if the statement refers to a table by an unqualified name,
+   and then a new table of the same name is created in a schema appearing
+   earlier in the search_path, no automatic re-parse will occur
+   since no object used in the statement changed.  However, if some other
+   change forces a re-parse, the new table will be referenced in subsequent
+   uses.
+  
+
   
    You can see all prepared statements available in the session by querying the
    pg_prepared_statements

diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index 13391689c7010de3c930d8a85823c8fd6fa46290..68693667b66caafe420d1ea4a1286d8db2a85205 100644 (file)
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -975,6 +975,20 @@ SPIPlanPtr SPI_prepare(const char * command, int 
    plans respectively.
   
 
+  
+   Although the main point of a prepared statement is to avoid repeated parse
+   analysis and planning of the statement, PostgreSQL will
+   force re-analysis and re-planning of the statement before using it
+   whenever database objects used in the statement have undergone
+   definitional (DDL) changes since the previous use of the prepared
+   statement.  Also, if the value of  changes
+   from one use to the next, the statement will be re-parsed using the new
+   search_path.  (This latter behavior is new as of
+   PostgreSQL 9.3.)  See 
+   linkend="sql-prepare"> for more information about the behavior of prepared
+   statements.
+  
+
   
    This function should only be called from a connected procedure.
   


diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c

index b256498d4511add3f55c35f07fecf6a92f19e763..ca4635dc51f41b050521d65cd601bccab90b6726 100644 (file)

--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3096,6 +3096,28 @@ CopyOverrideSearchPath(OverrideSearchPath *path)
    return result;
 }
 
+/*
+ * OverrideSearchPathMatchesCurrent - does path match current setting?
+ */
+bool
+OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
+{
+   /* Easiest way to do this is GetOverrideSearchPath() and compare */
+   bool        result;
+   OverrideSearchPath *cur;
+
+   cur = GetOverrideSearchPath(CurrentMemoryContext);
+   if (path->addCatalog == cur->addCatalog &&
+       path->addTemp == cur->addTemp &&
+       equal(path->schemas, cur->schemas))
+       result = true;
+   else
+       result = false;
+   list_free(cur->schemas);
+   pfree(cur);
+   return result;
+}
+
 /*
  * PushOverrideSearchPath - temporarily override the search path
  *


diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c

index cbc7c498d0d533b149fe4395992d9910a2d6b5cd..4630c44e7407c98a2de2df7bafc2753e78bfcfc6 100644 (file)

--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -15,13 +15,15 @@
  * that matches the event is marked invalid, as is its generic CachedPlan
  * if it has one.  When (and if) the next demand for a cached plan occurs,
  * parse analysis and rewrite is repeated to build a new valid query tree,
- * and then planning is performed as normal.
+ * and then planning is performed as normal.  We also force re-analysis and
+ * re-planning if the active search_path is different from the previous time.
  *
  * Note that if the sinval was a result of user DDL actions, parse analysis
  * could throw an error, for example if a column referenced by the query is
- * no longer present.  The creator of a cached plan can specify whether it
- * is allowable for the query to change output tupdesc on replan (this
- * could happen with "SELECT *" for example) --- if so, it's up to the
+ * no longer present.  Another possibility is for the query's output tupdesc
+ * to change (for instance "SELECT *" might expand differently than before).
+ * The creator of a cached plan can specify whether it is allowable for the
+ * query to change output tupdesc on replan --- if so, it's up to the
  * caller to notice changes and cope with them.
  *
  * Currently, we track exactly the dependencies of plans on relations and
@@ -174,11 +176,11 @@ CreateCachedPlan(Node *raw_parse_tree,
    plansource->cursor_options = 0;
    plansource->fixed_result = false;
    plansource->resultDesc = NULL;
-   plansource->search_path = NULL;
    plansource->context = source_context;
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
    plansource->query_context = NULL;
    plansource->gplan = NULL;
    plansource->is_oneshot = false;
@@ -239,11 +241,11 @@ CreateOneShotCachedPlan(Node *raw_parse_tree,
    plansource->cursor_options = 0;
    plansource->fixed_result = false;
    plansource->resultDesc = NULL;
-   plansource->search_path = NULL;
    plansource->context = CurrentMemoryContext;
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
    plansource->query_context = NULL;
    plansource->gplan = NULL;
    plansource->is_oneshot = true;
@@ -360,6 +362,14 @@ CompleteCachedPlan(CachedPlanSource *plansource,
                                   &plansource->relationOids,
                                   &plansource->invalItems);
 
+   /*
+    * Also save the current search_path in the query_context.  (This should
+    * not generate much extra cruft either, since almost certainly the path
+    * is already valid.)  Again, don't really need it for one-shot plans.
+    */
+   if (!plansource->is_oneshot)
+       plansource->search_path = GetOverrideSearchPath(querytree_context);
+
    /*
     * Save the final parameter types (or other parameter specification data)
     * into the source_context, as well as our other parameters.  Also save
@@ -383,12 +393,6 @@ CompleteCachedPlan(CachedPlanSource *plansource,
 
    MemoryContextSwitchTo(oldcxt);
 
-   /*
-    * Fetch current search_path into dedicated context, but do any
-    * recalculation work required in caller's context.
-    */
-   plansource->search_path = GetOverrideSearchPath(source_context);
-
    plansource->is_complete = true;
    plansource->is_valid = true;
 }
@@ -546,6 +550,23 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
        return NIL;
    }
 
+   /*
+    * If the query is currently valid, we should have a saved search_path ---
+    * check to see if that matches the current environment.  If not, we want
+    * to force replan.
+    */
+   if (plansource->is_valid)
+   {
+       Assert(plansource->search_path != NULL);
+       if (!OverrideSearchPathMatchesCurrent(plansource->search_path))
+       {
+           /* Invalidate the querytree and generic plan */
+           plansource->is_valid = false;
+           if (plansource->gplan)
+               plansource->gplan->is_valid = false;
+       }
+   }
+
    /*
     * If the query is currently valid, acquire locks on the referenced
     * objects; then check again.  We need to do it this way to cover the race
@@ -578,6 +599,7 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
 
    /*
     * Free the query_context.  We don't really expect MemoryContextDelete to
@@ -602,14 +624,6 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
     */
    Assert(plansource->is_complete);
 
-   /*
-    * Restore the search_path that was in use when the plan was made. See
-    * comments for PushOverrideSearchPath about limitations of this.
-    *
-    * (XXX is there anything else we really need to restore?)
-    */
-   PushOverrideSearchPath(plansource->search_path);
-
    /*
     * If a snapshot is already set (the normal case), we can just use that
     * for parsing/planning.  But if it isn't, install one.  Note: no point in
@@ -645,9 +659,6 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
    if (snapshot_set)
        PopActiveSnapshot();
 
-   /* Now we can restore current search path */
-   PopOverrideSearchPath();
-
    /*
     * Check or update the result tupdesc.  XXX should we use a weaker
     * condition than equalTupleDescs() here?
@@ -699,6 +710,13 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
                               &plansource->relationOids,
                               &plansource->invalItems);
 
+   /*
+    * Also save the current search_path in the query_context.  (This should
+    * not generate much extra cruft either, since almost certainly the path
+    * is already valid.)
+    */
+   plansource->search_path = GetOverrideSearchPath(querytree_context);
+
    MemoryContextSwitchTo(oldcxt);
 
    /* Now reparent the finished query_context and save the links */
@@ -848,20 +866,6 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
            qlist = plansource->query_list;
    }
 
-   /*
-    * Restore the search_path that was in use when the plan was made. See
-    * comments for PushOverrideSearchPath about limitations of this.
-    *
-    * (XXX is there anything else we really need to restore?)
-    *
-    * Note: it's a bit annoying to do this and snapshot-setting twice in the
-    * case where we have to do both re-analysis and re-planning.  However,
-    * until there's some evidence that the cost is actually meaningful
-    * compared to parse analysis + planning, I'm not going to contort the
-    * code enough to avoid that.
-    */
-   PushOverrideSearchPath(plansource->search_path);
-
    /*
     * If a snapshot is already set (the normal case), we can just use that
     * for planning.  But if it isn't, and we need one, install one.
@@ -894,9 +898,6 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
    if (snapshot_set)
        PopActiveSnapshot();
 
-   /* Now we can restore current search path */
-   PopOverrideSearchPath();
-
    /*
     * Normally we make a dedicated memory context for the CachedPlan and its
     * subsidiary data.  (It's probably not going to be large, but just in
@@ -1268,7 +1269,6 @@ CopyCachedPlan(CachedPlanSource *plansource)
        newsource->resultDesc = CreateTupleDescCopy(plansource->resultDesc);
    else
        newsource->resultDesc = NULL;
-   newsource->search_path = CopyOverrideSearchPath(plansource->search_path);
    newsource->context = source_context;
 
    querytree_context = AllocSetContextCreate(source_context,
@@ -1280,6 +1280,8 @@ CopyCachedPlan(CachedPlanSource *plansource)
    newsource->query_list = (List *) copyObject(plansource->query_list);
    newsource->relationOids = (List *) copyObject(plansource->relationOids);
    newsource->invalItems = (List *) copyObject(plansource->invalItems);
+   if (plansource->search_path)
+       newsource->search_path = CopyOverrideSearchPath(plansource->search_path);
    newsource->query_context = querytree_context;
 
    newsource->gplan = NULL;


diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h

index af82072edb7e90f243b0976337ae7d68fd2b9ce8..c37df8686ea2e835145d1de4b9ba358014b3a4f2 100644 (file)

--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -128,6 +128,7 @@ extern void ResetTempTableNamespace(void);
 
 extern OverrideSearchPath *GetOverrideSearchPath(MemoryContext context);
 extern OverrideSearchPath *CopyOverrideSearchPath(OverrideSearchPath *path);
+extern bool OverrideSearchPathMatchesCurrent(OverrideSearchPath *path);
 extern void PushOverrideSearchPath(OverrideSearchPath *newpath);
 extern void PopOverrideSearchPath(void);
 


diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h

index 8185427fc4b28ca3caca33f1d177439d626f708e..abaf9dc59c763b4b39978a49b52d5c85c19ed732 100644 (file)

--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -86,12 +86,13 @@ typedef struct CachedPlanSource
    int         cursor_options; /* cursor options used for planning */
    bool        fixed_result;   /* disallow change in result tupdesc? */
    TupleDesc   resultDesc;     /* result type; NULL = doesn't return tuples */
-   struct OverrideSearchPath *search_path;     /* saved search_path */
    MemoryContext context;      /* memory context holding all above */
    /* These fields describe the current analyzed-and-rewritten query tree: */
    List       *query_list;     /* list of Query nodes, or NIL if not valid */
    List       *relationOids;   /* OIDs of relations the queries depend on */
    List       *invalItems;     /* other dependencies, as PlanInvalItems */
+   struct OverrideSearchPath *search_path;     /* search_path used for
+                                                * parsing and planning */
    MemoryContext query_context;    /* context holding the above, or NULL */
    /* If we have a generic plan, this is a reference-counted link to it: */
    struct CachedPlan *gplan;   /* generic plan, or NULL if not valid */


diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out

index f0aa102334be6dce39c58ecd0135792e0ea46c90..864f70f7b54e7dbb021296f5213f023d53201dd0 100644 (file)

--- a/src/test/regress/expected/plancache.out
+++ b/src/test/regress/expected/plancache.out
@@ -163,7 +163,7 @@ select cache_test_2();
         10007
 (1 row)
 
---- Check that change of search_path is ignored by replans
+--- Check that change of search_path is honored when re-using cached plan
 create schema s1
   create table abc (f1 int);
 create schema s2
@@ -188,14 +188,14 @@ select f1 from abc;
 execute p1;
  f1  
 -----
- 123
+ 456
 (1 row)
 
 alter table s1.abc add column f2 float8;   -- force replan
 execute p1;
  f1  
 -----
- 123
+ 456
 (1 row)
 
 drop schema s1 cascade;


diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql

index 26848168f0631c1f423d32305e2a7d4d273428b0..bc2086166b985d7a3886d2899862443edee928bd 100644 (file)

--- a/src/test/regress/sql/plancache.sql
+++ b/src/test/regress/sql/plancache.sql
@@ -94,7 +94,7 @@ create or replace temp view v1 as
   select 2+2+4+(select max(unique1) from tenk1) as f1;
 select cache_test_2();
 
---- Check that change of search_path is ignored by replans
+--- Check that change of search_path is honored when re-using cached plan
 
 create schema s1
   create table abc (f1 int);

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index b256498d4511add3f55c35f07fecf6a92f19e763..ca4635dc51f41b050521d65cd601bccab90b6726 100644 (file)
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3096,6 +3096,28 @@ CopyOverrideSearchPath(OverrideSearchPath *path)
    return result;
 }
 
+/*
+ * OverrideSearchPathMatchesCurrent - does path match current setting?
+ */
+bool
+OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
+{
+   /* Easiest way to do this is GetOverrideSearchPath() and compare */
+   bool        result;
+   OverrideSearchPath *cur;
+
+   cur = GetOverrideSearchPath(CurrentMemoryContext);
+   if (path->addCatalog == cur->addCatalog &&
+       path->addTemp == cur->addTemp &&
+       equal(path->schemas, cur->schemas))
+       result = true;
+   else
+       result = false;
+   list_free(cur->schemas);
+   pfree(cur);
+   return result;
+}
+
 /*
  * PushOverrideSearchPath - temporarily override the search path
  *

diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index cbc7c498d0d533b149fe4395992d9910a2d6b5cd..4630c44e7407c98a2de2df7bafc2753e78bfcfc6 100644 (file)
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -15,13 +15,15 @@
  * that matches the event is marked invalid, as is its generic CachedPlan
  * if it has one.  When (and if) the next demand for a cached plan occurs,
  * parse analysis and rewrite is repeated to build a new valid query tree,
- * and then planning is performed as normal.
+ * and then planning is performed as normal.  We also force re-analysis and
+ * re-planning if the active search_path is different from the previous time.
  *
  * Note that if the sinval was a result of user DDL actions, parse analysis
  * could throw an error, for example if a column referenced by the query is
- * no longer present.  The creator of a cached plan can specify whether it
- * is allowable for the query to change output tupdesc on replan (this
- * could happen with "SELECT *" for example) --- if so, it's up to the
+ * no longer present.  Another possibility is for the query's output tupdesc
+ * to change (for instance "SELECT *" might expand differently than before).
+ * The creator of a cached plan can specify whether it is allowable for the
+ * query to change output tupdesc on replan --- if so, it's up to the
  * caller to notice changes and cope with them.
  *
  * Currently, we track exactly the dependencies of plans on relations and
@@ -174,11 +176,11 @@ CreateCachedPlan(Node *raw_parse_tree,
    plansource->cursor_options = 0;
    plansource->fixed_result = false;
    plansource->resultDesc = NULL;
-   plansource->search_path = NULL;
    plansource->context = source_context;
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
    plansource->query_context = NULL;
    plansource->gplan = NULL;
    plansource->is_oneshot = false;
@@ -239,11 +241,11 @@ CreateOneShotCachedPlan(Node *raw_parse_tree,
    plansource->cursor_options = 0;
    plansource->fixed_result = false;
    plansource->resultDesc = NULL;
-   plansource->search_path = NULL;
    plansource->context = CurrentMemoryContext;
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
    plansource->query_context = NULL;
    plansource->gplan = NULL;
    plansource->is_oneshot = true;
@@ -360,6 +362,14 @@ CompleteCachedPlan(CachedPlanSource *plansource,
                                   &plansource->relationOids,
                                   &plansource->invalItems);
 
+   /*
+    * Also save the current search_path in the query_context.  (This should
+    * not generate much extra cruft either, since almost certainly the path
+    * is already valid.)  Again, don't really need it for one-shot plans.
+    */
+   if (!plansource->is_oneshot)
+       plansource->search_path = GetOverrideSearchPath(querytree_context);
+
    /*
     * Save the final parameter types (or other parameter specification data)
     * into the source_context, as well as our other parameters.  Also save
@@ -383,12 +393,6 @@ CompleteCachedPlan(CachedPlanSource *plansource,
 
    MemoryContextSwitchTo(oldcxt);
 
-   /*
-    * Fetch current search_path into dedicated context, but do any
-    * recalculation work required in caller's context.
-    */
-   plansource->search_path = GetOverrideSearchPath(source_context);
-
    plansource->is_complete = true;
    plansource->is_valid = true;
 }
@@ -546,6 +550,23 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
        return NIL;
    }
 
+   /*
+    * If the query is currently valid, we should have a saved search_path ---
+    * check to see if that matches the current environment.  If not, we want
+    * to force replan.
+    */
+   if (plansource->is_valid)
+   {
+       Assert(plansource->search_path != NULL);
+       if (!OverrideSearchPathMatchesCurrent(plansource->search_path))
+       {
+           /* Invalidate the querytree and generic plan */
+           plansource->is_valid = false;
+           if (plansource->gplan)
+               plansource->gplan->is_valid = false;
+       }
+   }
+
    /*
     * If the query is currently valid, acquire locks on the referenced
     * objects; then check again.  We need to do it this way to cover the race
@@ -578,6 +599,7 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
    plansource->query_list = NIL;
    plansource->relationOids = NIL;
    plansource->invalItems = NIL;
+   plansource->search_path = NULL;
 
    /*
     * Free the query_context.  We don't really expect MemoryContextDelete to
@@ -602,14 +624,6 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
     */
    Assert(plansource->is_complete);
 
-   /*
-    * Restore the search_path that was in use when the plan was made. See
-    * comments for PushOverrideSearchPath about limitations of this.
-    *
-    * (XXX is there anything else we really need to restore?)
-    */
-   PushOverrideSearchPath(plansource->search_path);
-
    /*
     * If a snapshot is already set (the normal case), we can just use that
     * for parsing/planning.  But if it isn't, install one.  Note: no point in
@@ -645,9 +659,6 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
    if (snapshot_set)
        PopActiveSnapshot();
 
-   /* Now we can restore current search path */
-   PopOverrideSearchPath();
-
    /*
     * Check or update the result tupdesc.  XXX should we use a weaker
     * condition than equalTupleDescs() here?
@@ -699,6 +710,13 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
                               &plansource->relationOids,
                               &plansource->invalItems);
 
+   /*
+    * Also save the current search_path in the query_context.  (This should
+    * not generate much extra cruft either, since almost certainly the path
+    * is already valid.)
+    */
+   plansource->search_path = GetOverrideSearchPath(querytree_context);
+
    MemoryContextSwitchTo(oldcxt);
 
    /* Now reparent the finished query_context and save the links */
@@ -848,20 +866,6 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
            qlist = plansource->query_list;
    }
 
-   /*
-    * Restore the search_path that was in use when the plan was made. See
-    * comments for PushOverrideSearchPath about limitations of this.
-    *
-    * (XXX is there anything else we really need to restore?)
-    *
-    * Note: it's a bit annoying to do this and snapshot-setting twice in the
-    * case where we have to do both re-analysis and re-planning.  However,
-    * until there's some evidence that the cost is actually meaningful
-    * compared to parse analysis + planning, I'm not going to contort the
-    * code enough to avoid that.
-    */
-   PushOverrideSearchPath(plansource->search_path);
-
    /*
     * If a snapshot is already set (the normal case), we can just use that
     * for planning.  But if it isn't, and we need one, install one.
@@ -894,9 +898,6 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
    if (snapshot_set)
        PopActiveSnapshot();
 
-   /* Now we can restore current search path */
-   PopOverrideSearchPath();
-
    /*
     * Normally we make a dedicated memory context for the CachedPlan and its
     * subsidiary data.  (It's probably not going to be large, but just in
@@ -1268,7 +1269,6 @@ CopyCachedPlan(CachedPlanSource *plansource)
        newsource->resultDesc = CreateTupleDescCopy(plansource->resultDesc);
    else
        newsource->resultDesc = NULL;
-   newsource->search_path = CopyOverrideSearchPath(plansource->search_path);
    newsource->context = source_context;
 
    querytree_context = AllocSetContextCreate(source_context,
@@ -1280,6 +1280,8 @@ CopyCachedPlan(CachedPlanSource *plansource)
    newsource->query_list = (List *) copyObject(plansource->query_list);
    newsource->relationOids = (List *) copyObject(plansource->relationOids);
    newsource->invalItems = (List *) copyObject(plansource->invalItems);
+   if (plansource->search_path)
+       newsource->search_path = CopyOverrideSearchPath(plansource->search_path);
    newsource->query_context = querytree_context;
 
    newsource->gplan = NULL;

diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index af82072edb7e90f243b0976337ae7d68fd2b9ce8..c37df8686ea2e835145d1de4b9ba358014b3a4f2 100644 (file)
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -128,6 +128,7 @@ extern void ResetTempTableNamespace(void);
 
 extern OverrideSearchPath *GetOverrideSearchPath(MemoryContext context);
 extern OverrideSearchPath *CopyOverrideSearchPath(OverrideSearchPath *path);
+extern bool OverrideSearchPathMatchesCurrent(OverrideSearchPath *path);
 extern void PushOverrideSearchPath(OverrideSearchPath *newpath);
 extern void PopOverrideSearchPath(void);
 

diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 8185427fc4b28ca3caca33f1d177439d626f708e..abaf9dc59c763b4b39978a49b52d5c85c19ed732 100644 (file)
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -86,12 +86,13 @@ typedef struct CachedPlanSource
    int         cursor_options; /* cursor options used for planning */
    bool        fixed_result;   /* disallow change in result tupdesc? */
    TupleDesc   resultDesc;     /* result type; NULL = doesn't return tuples */
-   struct OverrideSearchPath *search_path;     /* saved search_path */
    MemoryContext context;      /* memory context holding all above */
    /* These fields describe the current analyzed-and-rewritten query tree: */
    List       *query_list;     /* list of Query nodes, or NIL if not valid */
    List       *relationOids;   /* OIDs of relations the queries depend on */
    List       *invalItems;     /* other dependencies, as PlanInvalItems */
+   struct OverrideSearchPath *search_path;     /* search_path used for
+                                                * parsing and planning */
    MemoryContext query_context;    /* context holding the above, or NULL */
    /* If we have a generic plan, this is a reference-counted link to it: */
    struct CachedPlan *gplan;   /* generic plan, or NULL if not valid */

diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out
index f0aa102334be6dce39c58ecd0135792e0ea46c90..864f70f7b54e7dbb021296f5213f023d53201dd0 100644 (file)
--- a/src/test/regress/expected/plancache.out
+++ b/src/test/regress/expected/plancache.out
@@ -163,7 +163,7 @@ select cache_test_2();
         10007
 (1 row)
 
---- Check that change of search_path is ignored by replans
+--- Check that change of search_path is honored when re-using cached plan
 create schema s1
   create table abc (f1 int);
 create schema s2
@@ -188,14 +188,14 @@ select f1 from abc;
 execute p1;
  f1  
 -----
- 123
+ 456
 (1 row)
 
 alter table s1.abc add column f2 float8;   -- force replan
 execute p1;
  f1  
 -----
- 123
+ 456
 (1 row)
 
 drop schema s1 cascade;

diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql
index 26848168f0631c1f423d32305e2a7d4d273428b0..bc2086166b985d7a3886d2899862443edee928bd 100644 (file)
--- a/src/test/regress/sql/plancache.sql
+++ b/src/test/regress/sql/plancache.sql
@@ -94,7 +94,7 @@ create or replace temp view v1 as
   select 2+2+4+(select max(unique1) from tenk1) as f1;
 select cache_test_2();
 
---- Check that change of search_path is ignored by replans
+--- Check that change of search_path is honored when re-using cached plan
 
 create schema s1
   create table abc (f1 int);