Ensure that a cursor has an immutable snapshot throughout its lifespan.

The old coding was using a regular snapshot, referenced elsewhere, that was
subject to having its command counter updated.  Fix by creating a private copy
of the snapshot exclusively for the cursor.

Backpatch to 8.4, which is when the bug was introduced during the snapshot
management rewrite.

diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 9ca2e84c52517a5dd0789d58420857f41b2456a5..e89e371c37d75d29b5d7b5dbdc07652e366a2457 100644 (file)
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -47,6 +47,7 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
        DeclareCursorStmt *cstmt = (DeclareCursorStmt *) stmt->utilityStmt;
        Portal          portal;
        MemoryContext oldContext;
+       Snapshot        snapshot;
 
        if (cstmt == NULL || !IsA(cstmt, DeclareCursorStmt))
                elog(ERROR, "PerformCursorOpen called for non-cursor query");
@@ -118,10 +119,18 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
                        portal->cursorOptions |= CURSOR_OPT_NO_SCROLL;
        }
 
+       /*
+        * Set up snapshot for portal.  Note that we need a fresh, independent copy
+        * of the snapshot because we don't want it to be modified by future
+        * CommandCounterIncrement calls.  We do not register it, because
+        * portalmem.c will take care of that internally.
+        */
+       snapshot = CopySnapshot(GetActiveSnapshot());
+
        /*
         * Start execution, inserting parameters if any.
         */
-       PortalStart(portal, params, GetActiveSnapshot());
+       PortalStart(portal, params, snapshot);
 
        Assert(portal->strategy == PORTAL_ONE_SELECT);
 

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 12b55d5745512667e86c07f9d92cfa016c8fef12..4cd7b0b2120208bdc61af9ac2a8b688503ed44a0 100644 (file)
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1211,10 +1211,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
                }
        }
 
-       /*
-        * Set up the snapshot to use.  (PortalStart will do PushActiveSnapshot,
-        * so we skip that here.)
-        */
+       /* Set up the snapshot to use. */
        if (read_only)
                snapshot = GetActiveSnapshot();
        else

diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 85065d3572838f174a6702d719baa33e82f98f97..95ff4cba271250f3ff98136c3918f04c4e20509a 100644 (file)
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -104,7 +104,6 @@ bool                FirstSnapshotSet = false;
 static bool registered_serializable = false;
 
 
-static Snapshot CopySnapshot(Snapshot snapshot);
 static void FreeSnapshot(Snapshot snapshot);
 static void SnapshotResetXmin(void);
 
@@ -192,7 +191,7 @@ SnapshotSetCommandId(CommandId curcid)
  * The copy is palloc'd in TopTransactionContext and has initial refcounts set
  * to 0.  The returned snapshot has the copied flag set.
  */
-static Snapshot
+Snapshot
 CopySnapshot(Snapshot snapshot)
 {
        Snapshot        newsnap;

diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index 6ccb44fd5b4d7ee80d2b47228e0797402819f53d..737b3c7117b18c4735faa0727a130ca84ab3dbf6 100644 (file)
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -26,6 +26,7 @@ extern TransactionId RecentGlobalXmin;
 extern Snapshot GetTransactionSnapshot(void);
 extern Snapshot GetLatestSnapshot(void);
 extern void SnapshotSetCommandId(CommandId curcid);
+extern Snapshot CopySnapshot(Snapshot snapshot);
 
 extern void PushActiveSnapshot(Snapshot snapshot);
 extern void PushUpdatedSnapshot(Snapshot snapshot);

diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index 95dcea5a1d9ebbd2c24c7d13563dfc7c68a4aaf7..be7348d6b2adaba4f4357672011d9cffa48a827f 100644 (file)
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1242,3 +1242,18 @@ FETCH FROM c1;
 DELETE FROM ucview WHERE CURRENT OF c1; -- fail, views not supported
 ERROR:  WHERE CURRENT OF on a view is not implemented
 ROLLBACK;
+-- Make sure snapshot management works okay, per bug report in
+-- [email protected]
+BEGIN; 
+SET TRANSACTION ISOLATION LEVEL SERIALIZABLE; 
+CREATE TABLE cursor (a int); 
+INSERT INTO cursor VALUES (1); 
+DECLARE c1 NO SCROLL CURSOR FOR SELECT * FROM cursor FOR UPDATE; 
+UPDATE cursor SET a = 2; 
+FETCH ALL FROM c1; 
+ a 
+---
+(0 rows)
+
+COMMIT; 
+DROP TABLE cursor;

diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql
index 4265aaa43cf110bf9efb49c78103f248982b0e84..585a7c25ea7514666d5ef395c5ba2e6efcf2a293 100644 (file)
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -458,3 +458,15 @@ DECLARE c1 CURSOR FOR SELECT * FROM ucview;
 FETCH FROM c1;
 DELETE FROM ucview WHERE CURRENT OF c1; -- fail, views not supported
 ROLLBACK;
+
+-- Make sure snapshot management works okay, per bug report in
+-- [email protected]
+BEGIN; 
+SET TRANSACTION ISOLATION LEVEL SERIALIZABLE; 
+CREATE TABLE cursor (a int); 
+INSERT INTO cursor VALUES (1); 
+DECLARE c1 NO SCROLL CURSOR FOR SELECT * FROM cursor FOR UPDATE; 
+UPDATE cursor SET a = 2; 
+FETCH ALL FROM c1; 
+COMMIT; 
+DROP TABLE cursor;