Repair bug that allowed RevalidateCachedPlan to attempt to rebuild a cached

plan before the effects of DDL executed in an immediately prior SPI operation
had been absorbed.  Per report from Chris Wood.

This patch has an unpleasant side effect of causing the number of
CommandCounterIncrement()s done by a typical plpgsql function to
approximately double.  Amelioration of the consequences of that
will be undertaken in a separate patch.

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 3d72fa20a5506b80ba8c11a082ea4373b740a50b..a5601ecdfca44bd39ea3e733c7a548fa4aa63c90 100644 (file)
--- 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.184 2007/11/15 21:14:35 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.185 2007/11/30 18:38:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1374,14 +1374,6 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan)
        int                     nargs = plan->nargs;
        int                     cursor_options = plan->cursor_options;
 
-       /*
-        * Increment CommandCounter to see changes made by now.  We must do this
-        * to be sure of seeing any schema changes made by a just-preceding SPI
-        * command.  (But we don't bother advancing the snapshot, since the
-        * planner generally operates under SnapshotNow rules anyway.)
-        */
-       CommandCounterIncrement();
-
        /*
         * Setup error traceback support for ereport()
         */
@@ -1662,6 +1654,14 @@ _SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
                        if (cplan)
                                ReleaseCachedPlan(cplan, true);
                        cplan = NULL;
+
+                       /*
+                        * If not read-only mode, advance the command counter after the
+                        * last command.  This ensures that its effects are visible, in
+                        * case it was DDL that would affect the next CachedPlanSource.
+                        */
+                       if (!read_only)
+                               CommandCounterIncrement();
                }
 
 fail:

diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out
index d7d7be92529aee67b0ab7b08e4de39e5157749b2..7ee11921c25994d0fdea2f285cbf8be3a5523595 100644 (file)
--- a/src/test/regress/expected/plancache.out
+++ b/src/test/regress/expected/plancache.out
@@ -202,6 +202,7 @@ drop schema s1 cascade;
 NOTICE:  drop cascades to table s1.abc
 drop schema s2 cascade;
 NOTICE:  drop cascades to table abc
+reset search_path;
 -- Check that invalidation deals with regclass constants
 create temp sequence seq;
 prepare p2 as select nextval('seq');
@@ -219,3 +220,42 @@ execute p2;
        1
 (1 row)
 
+-- Check DDL via SPI, immediately followed by SPI plan re-use
+-- (bug in original coding)
+create function cachebug() returns void as $$
+declare r int;
+begin
+  drop table if exists temptable cascade;
+  create temp table temptable as select * from generate_series(1,3) as f1;
+  create temp view vv as select * from temptable;
+  for r in select * from vv loop
+    raise notice '%', r;
+  end loop;
+end$$ language plpgsql;
+select cachebug();
+NOTICE:  table "temptable" does not exist, skipping
+CONTEXT:  SQL statement "drop table if exists temptable cascade"
+PL/pgSQL function "cachebug" line 3 at SQL statement
+NOTICE:  1
+NOTICE:  2
+NOTICE:  3
+ cachebug 
+----------
+ 
+(1 row)
+
+select cachebug();
+NOTICE:  drop cascades to rule _RETURN on view vv
+CONTEXT:  SQL statement "drop table if exists temptable cascade"
+PL/pgSQL function "cachebug" line 3 at SQL statement
+NOTICE:  drop cascades to view vv
+CONTEXT:  SQL statement "drop table if exists temptable cascade"
+PL/pgSQL function "cachebug" line 3 at SQL statement
+NOTICE:  1
+NOTICE:  2
+NOTICE:  3
+ cachebug 
+----------
+ 
+(1 row)
+

diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql
index fc57279d985c702476509b7f2bda227ac2e37dd2..26848168f0631c1f423d32305e2a7d4d273428b0 100644 (file)
--- a/src/test/regress/sql/plancache.sql
+++ b/src/test/regress/sql/plancache.sql
@@ -124,6 +124,8 @@ execute p1;
 drop schema s1 cascade;
 drop schema s2 cascade;
 
+reset search_path;
+
 -- Check that invalidation deals with regclass constants
 
 create temp sequence seq;
@@ -137,3 +139,20 @@ drop sequence seq;
 create temp sequence seq;
 
 execute p2;
+
+-- Check DDL via SPI, immediately followed by SPI plan re-use
+-- (bug in original coding)
+
+create function cachebug() returns void as $$
+declare r int;
+begin
+  drop table if exists temptable cascade;
+  create temp table temptable as select * from generate_series(1,3) as f1;
+  create temp view vv as select * from temptable;
+  for r in select * from vv loop
+    raise notice '%', r;
+  end loop;
+end$$ language plpgsql;
+
+select cachebug();
+select cachebug();