Skip to content

Commit caa4cfa

Browse files
alvherrealvherre
committed
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.
1 parent fabf75c commit caa4cfa

File tree

6 files changed

+44
-11
lines changed

6 files changed

+44
-11
lines changed

‎src/backend/commands/portalcmds.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.79 2009/06/11 14:48:56 momjian Exp $
17+
* $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.80 2009/10/02 17:57:29 alvherre Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -47,6 +47,7 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
4747
DeclareCursorStmt *cstmt = (DeclareCursorStmt *) stmt->utilityStmt;
4848
Portal portal;
4949
MemoryContext oldContext;
50+
Snapshot snapshot;
5051

5152
if (cstmt == NULL || !IsA(cstmt, DeclareCursorStmt))
5253
elog(ERROR, "PerformCursorOpen called for non-cursor query");
@@ -118,10 +119,18 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
118119
portal->cursorOptions |= CURSOR_OPT_NO_SCROLL;
119120
}
120121

122+
/*
123+
* Set up snapshot for portal. Note that we need a fresh, independent copy
124+
* of the snapshot because we don't want it to be modified by future
125+
* CommandCounterIncrement calls. We do not register it, because
126+
* portalmem.c will take care of that internally.
127+
*/
128+
snapshot = CopySnapshot(GetActiveSnapshot());
129+
121130
/*
122131
* Start execution, inserting parameters if any.
123132
*/
124-
PortalStart(portal, params, GetActiveSnapshot());
133+
PortalStart(portal, params, snapshot);
125134

126135
Assert(portal->strategy == PORTAL_ONE_SELECT);
127136

‎src/backend/executor/spi.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.208 2009/06/11 14:48:57 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.209 2009/10/02 17:57:30 alvherre Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1211,10 +1211,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
12111211
}
12121212
}
12131213

1214-
/*
1215-
* Set up the snapshot to use. (PortalStart will do PushActiveSnapshot,
1216-
* so we skip that here.)
1217-
*/
1214+
/* Set up the snapshot to use. */
12181215
if (read_only)
12191216
snapshot = GetActiveSnapshot();
12201217
else

‎src/backend/utils/time/snapmgr.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* Portions Copyright (c) 1994, Regents of the University of California
2020
*
2121
* IDENTIFICATION
22-
* $PostgreSQL: pgsql/src/backend/utils/time/snapmgr.c,v 1.10 2009/06/11 14:49:06 momjian Exp $
22+
* $PostgreSQL: pgsql/src/backend/utils/time/snapmgr.c,v 1.11 2009/10/02 17:57:30 alvherre Exp $
2323
*
2424
*-------------------------------------------------------------------------
2525
*/
@@ -104,7 +104,6 @@ bool FirstSnapshotSet = false;
104104
static bool registered_serializable = false;
105105

106106

107-
static Snapshot CopySnapshot(Snapshot snapshot);
108107
static void FreeSnapshot(Snapshot snapshot);
109108
static void SnapshotResetXmin(void);
110109

@@ -192,7 +191,7 @@ SnapshotSetCommandId(CommandId curcid)
192191
* The copy is palloc'd in TopTransactionContext and has initial refcounts set
193192
* to 0. The returned snapshot has the copied flag set.
194193
*/
195-
static Snapshot
194+
Snapshot
196195
CopySnapshot(Snapshot snapshot)
197196
{
198197
Snapshot newsnap;

‎src/include/utils/snapmgr.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $PostgreSQL: pgsql/src/include/utils/snapmgr.h,v 1.5 2009/06/11 14:49:13 momjian Exp $
9+
* $PostgreSQL: pgsql/src/include/utils/snapmgr.h,v 1.6 2009/10/02 17:57:30 alvherre Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -26,6 +26,7 @@ extern TransactionId RecentGlobalXmin;
2626
extern Snapshot GetTransactionSnapshot(void);
2727
extern Snapshot GetLatestSnapshot(void);
2828
extern void SnapshotSetCommandId(CommandId curcid);
29+
extern Snapshot CopySnapshot(Snapshot snapshot);
2930

3031
extern void PushActiveSnapshot(Snapshot snapshot);
3132
extern void PushUpdatedSnapshot(Snapshot snapshot);

‎src/test/regress/expected/portals.out

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,3 +1242,18 @@ FETCH FROM c1;
12421242
DELETE FROM ucview WHERE CURRENT OF c1; -- fail, views not supported
12431243
ERROR: WHERE CURRENT OF on a view is not implemented
12441244
ROLLBACK;
1245+
-- Make sure snapshot management works okay, per bug report in
1246+
-- 235395b90909301035v7228ce63q392931f15aa74b31@mail.gmail.com
1247+
BEGIN;
1248+
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
1249+
CREATE TABLE cursor (a int);
1250+
INSERT INTO cursor VALUES (1);
1251+
DECLARE c1 NO SCROLL CURSOR FOR SELECT * FROM cursor FOR UPDATE;
1252+
UPDATE cursor SET a = 2;
1253+
FETCH ALL FROM c1;
1254+
a
1255+
---
1256+
(0 rows)
1257+
1258+
COMMIT;
1259+
DROP TABLE cursor;

‎src/test/regress/sql/portals.sql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,3 +458,15 @@ DECLARE c1 CURSOR FOR SELECT * FROM ucview;
458458
FETCH FROM c1;
459459
DELETE FROM ucview WHERE CURRENT OF c1; -- fail, views not supported
460460
ROLLBACK;
461+
462+
-- Make sure snapshot management works okay, per bug report in
463+
-- 235395b90909301035v7228ce63q392931f15aa74b31@mail.gmail.com
464+
BEGIN;
465+
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
466+
CREATE TABLE cursor (a int);
467+
INSERT INTO cursor VALUES (1);
468+
DECLARE c1 NO SCROLL CURSOR FOR SELECT * FROM cursor FOR UPDATE;
469+
UPDATE cursor SET a = 2;
470+
FETCH ALL FROM c1;
471+
COMMIT;
472+
DROP TABLE cursor;

0 commit comments

Comments
 (0)