Revert patch for taking fewer snapshots.

This reverts commit d573e239f03506920938bf0be56c868d9c3416da, "Take fewer
snapshots".  While that seemed like a good idea at the time, it caused
execution to use a snapshot that had been acquired before locking any of
the tables mentioned in the query.  This created user-visible anomalies
that were not present in any prior release of Postgres, as reported by
Tomas Vondra.  While this whole area could do with a redesign (since there
are related cases that have anomalies anyway), it doesn't seem likely that
any future patch would be reasonably back-patchable; and we don't want 9.2
to exhibit a behavior that's subtly unlike either past or future releases.
Hence, revert to prior code while we rethink the problem.

diff --git a/doc/src/sgml/release-9.2.sgml b/doc/src/sgml/release-9.2.sgml
index 840e7a7a0d9acdfe3b8655a9f024f5a67cf49af5..c641606fd52010611d292bba95ebc81fcff1df71 100644 (file)
--- a/doc/src/sgml/release-9.2.sgml
+++ b/doc/src/sgml/release-9.2.sgml
@@ -809,13 +809,6 @@
        </para>
       </listitem>
 
-      <listitem>
-       <para>
-        Take fewer <acronym>MVCC</acronym> snapshots
-        (Robert Haas)
-       </para>
-      </listitem>
-
       <listitem>
        <para>
         Make the number of CLOG buffers scale based on <link

diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index e402042332569b5599de18fd7171c11e65038298..b5ce87a29d284bc7923ab47a87939ece94720214 100644 (file)
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -121,7 +121,7 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
        /*
         * Start execution, inserting parameters if any.
         */
-       PortalStart(portal, params, 0, true);
+       PortalStart(portal, params, 0, GetActiveSnapshot());
 
        Assert(portal->strategy == PORTAL_ONE_SELECT);
 

diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 9f993de6f1a80465631fb1ae041ce415c25f9ca6..91ef5507e2c49f89191d69be6883f7426c78807c 100644 (file)
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -289,7 +289,7 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause,
        /*
         * Run the portal as appropriate.
         */
-       PortalStart(portal, paramLI, eflags, true);
+       PortalStart(portal, paramLI, eflags, GetActiveSnapshot());
 
        (void) PortalRun(portal, count, false, dest, dest, completionTag);
 

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index bf8c4c711369a4637d936e1e2b7d88b22083891c..416a2c4f3bbe2132ed6d66f6eeaee9067e915d9b 100644 (file)
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1127,6 +1127,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
        CachedPlan *cplan;
        List       *stmt_list;
        char       *query_string;
+       Snapshot        snapshot;
        MemoryContext oldcontext;
        Portal          portal;
 
@@ -1269,6 +1270,15 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
                }
        }
 
+       /* Set up the snapshot to use. */
+       if (read_only)
+               snapshot = GetActiveSnapshot();
+       else
+       {
+               CommandCounterIncrement();
+               snapshot = GetTransactionSnapshot();
+       }
+
        /*
         * If the plan has parameters, copy them into the portal.  Note that this
         * must be done after revalidating the plan, because in dynamic parameter
@@ -1284,13 +1294,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
        /*
         * Start portal execution.
         */
-       if (read_only)
-               PortalStart(portal, paramLI, 0, true);
-       else
-       {
-               CommandCounterIncrement();
-               PortalStart(portal, paramLI, 0, false);
-       }
+       PortalStart(portal, paramLI, 0, snapshot);
 
        Assert(portal->strategy != PORTAL_MULTI_QUERY);
 

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 585db1af89cd37ba7e8f5327bbd992c45beaf288..1fd39f2d99bc0aa61c3ecd7a604c221e51b7e3c8 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -976,6 +976,10 @@ exec_simple_query(const char *query_string)
 
                plantree_list = pg_plan_queries(querytree_list, 0, NULL);
 
+               /* Done with the snapshot used for parsing/planning */
+               if (snapshot_set)
+                       PopActiveSnapshot();
+
                /* If we got a cancel signal in analysis or planning, quit */
                CHECK_FOR_INTERRUPTS();
 
@@ -1000,19 +1004,9 @@ exec_simple_query(const char *query_string)
                                                  NULL);
 
                /*
-                * Start the portal.
-                *
-                * If we took a snapshot for parsing/planning, the portal may be able
-                * to reuse it for the execution phase.  Currently, this will only
-                * happen in PORTAL_ONE_SELECT mode.  But even if PortalStart doesn't
-                * end up being able to do this, keeping the parse/plan snapshot
-                * around until after we start the portal doesn't cost much.
+                * Start the portal.  No parameters here.
                 */
-               PortalStart(portal, NULL, 0, snapshot_set);
-
-               /* Done with the snapshot used for parsing/planning */
-               if (snapshot_set)
-                       PopActiveSnapshot();
+               PortalStart(portal, NULL, 0, InvalidSnapshot);
 
                /*
                 * Select the appropriate output format: text unless we are doing a
@@ -1735,19 +1729,15 @@ exec_bind_message(StringInfo input_message)
                                          cplan->stmt_list,
                                          cplan);
 
-       /*
-        * And we're ready to start portal execution.
-        *
-        * If we took a snapshot for parsing/planning, we'll try to reuse it for
-        * query execution (currently, reuse will only occur if PORTAL_ONE_SELECT
-        * mode is chosen).
-        */
-       PortalStart(portal, params, 0, snapshot_set);
-
        /* Done with the snapshot used for parameter I/O and parsing/planning */
        if (snapshot_set)
                PopActiveSnapshot();
 
+       /*
+        * And we're ready to start portal execution.
+        */
+       PortalStart(portal, params, 0, InvalidSnapshot);
+
        /*
         * Apply the result format requests to the portal.
         */

diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 2cb9a8ee2faf5fdda78be708febb25d4688757ea..8ad0aaa55ec00b515d107be8f16f286c9d66c03a 100644 (file)
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -447,18 +447,17 @@ FetchStatementTargetList(Node *stmt)
  * currently only honored for PORTAL_ONE_SELECT portals).  Most callers
  * should simply pass zero.
  *
- * The use_active_snapshot parameter is currently used only for
- * PORTAL_ONE_SELECT portals.  If it is true, the active snapshot will
- * be used when starting up the executor; if false, a new snapshot will
- * be taken.  This is used both for cursors and to avoid taking an entirely
- * new snapshot when it isn't necessary.
+ * The caller can optionally pass a snapshot to be used; pass InvalidSnapshot
+ * for the normal behavior of setting a new snapshot.  This parameter is
+ * presently ignored for non-PORTAL_ONE_SELECT portals (it's only intended
+ * to be used for cursors).
  *
  * On return, portal is ready to accept PortalRun() calls, and the result
  * tupdesc (if any) is known.
  */
 void
 PortalStart(Portal portal, ParamListInfo params,
-                       int eflags, bool use_active_snapshot)
+                       int eflags, Snapshot snapshot)
 {
        Portal          saveActivePortal;
        ResourceOwner saveResourceOwner;
@@ -500,8 +499,8 @@ PortalStart(Portal portal, ParamListInfo params,
                        case PORTAL_ONE_SELECT:
 
                                /* Must set snapshot before starting executor. */
-                               if (use_active_snapshot)
-                                       PushActiveSnapshot(GetActiveSnapshot());
+                               if (snapshot)
+                                       PushActiveSnapshot(snapshot);
                                else
                                        PushActiveSnapshot(GetTransactionSnapshot());
 

diff --git a/src/include/tcop/pquery.h b/src/include/tcop/pquery.h
index 22aad2e96cc230d2c1cdfc7a44332f77b36b942e..c77bb8be9487006f5a82060a0ad2cf0ea638d124 100644 (file)
--- a/src/include/tcop/pquery.h
+++ b/src/include/tcop/pquery.h
@@ -28,7 +28,7 @@ extern List *FetchPortalTargetList(Portal portal);
 extern List *FetchStatementTargetList(Node *stmt);
 
 extern void PortalStart(Portal portal, ParamListInfo params,
-                       int eflags, bool use_active_snapshot);
+                       int eflags, Snapshot snapshot);
 
 extern void PortalSetResultFormat(Portal portal, int nFormats,
                                          int16 *formats);