diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index ad12395bf2..278d747e21 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -4842,6 +4842,7 @@ CommitSubTransaction(void) AfterTriggerEndSubXact(true); AtSubCommit_Portals(s->subTransactionId, s->parent->subTransactionId, + s->parent->nestingLevel, s->parent->curTransactionOwner); AtEOSubXact_LargeObject(true, s->subTransactionId, s->parent->subTransactionId); diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 5781dcde7c..a8f65ca165 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -480,7 +480,9 @@ PortalStart(Portal portal, ParamListInfo params, * We could remember the snapshot in portal->portalSnapshot, * but presently there seems no need to, as this code path * cannot be used for non-atomic execution. Hence there can't - * be any commit/abort that might destroy the snapshot. + * be any commit/abort that might destroy the snapshot. Since + * we don't do that, there's also no need to force a + * non-default nesting level for the snapshot. */ /* @@ -1134,9 +1136,15 @@ PortalRunUtility(Portal portal, PlannedStmt *pstmt, snapshot = RegisterSnapshot(snapshot); portal->holdSnapshot = snapshot; } - /* In any case, make the snapshot active and remember it in portal */ - PushActiveSnapshot(snapshot); - /* PushActiveSnapshot might have copied the snapshot */ + + /* + * In any case, make the snapshot active and remember it in portal. + * Because the portal now references the snapshot, we must tell + * snapmgr.c that the snapshot belongs to the portal's transaction + * level, else we risk portalSnapshot becoming a dangling pointer. + */ + PushActiveSnapshotWithLevel(snapshot, portal->createLevel); + /* PushActiveSnapshotWithLevel might have copied the snapshot */ portal->portalSnapshot = GetActiveSnapshot(); } else @@ -1786,8 +1794,13 @@ EnsurePortalSnapshotExists(void) elog(ERROR, "cannot execute SQL without an outer snapshot or portal"); Assert(portal->portalSnapshot == NULL); - /* Create a new snapshot and make it active */ - PushActiveSnapshot(GetTransactionSnapshot()); - /* PushActiveSnapshot might have copied the snapshot */ + /* + * Create a new snapshot, make it active, and remember it in portal. + * Because the portal now references the snapshot, we must tell snapmgr.c + * that the snapshot belongs to the portal's transaction level, else we + * risk portalSnapshot becoming a dangling pointer. + */ + PushActiveSnapshotWithLevel(GetTransactionSnapshot(), portal->createLevel); + /* PushActiveSnapshotWithLevel might have copied the snapshot */ portal->portalSnapshot = GetActiveSnapshot(); } diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index a34abbd80c..11ee752f84 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -210,6 +210,7 @@ CreatePortal(const char *name, bool allowDup, bool dupSilent) portal->cleanup = PortalCleanup; portal->createSubid = GetCurrentSubTransactionId(); portal->activeSubid = portal->createSubid; + portal->createLevel = GetCurrentTransactionNestLevel(); portal->strategy = PORTAL_MULTI_QUERY; portal->cursorOptions = CURSOR_OPT_NO_SCROLL; portal->atStart = true; @@ -657,6 +658,7 @@ HoldPortal(Portal portal) */ portal->createSubid = InvalidSubTransactionId; portal->activeSubid = InvalidSubTransactionId; + portal->createLevel = 0; } /* @@ -940,6 +942,7 @@ PortalErrorCleanup(void) void AtSubCommit_Portals(SubTransactionId mySubid, SubTransactionId parentSubid, + int parentLevel, ResourceOwner parentXactOwner) { HASH_SEQ_STATUS status; @@ -954,6 +957,7 @@ AtSubCommit_Portals(SubTransactionId mySubid, if (portal->createSubid == mySubid) { portal->createSubid = parentSubid; + portal->createLevel = parentLevel; if (portal->resowner) ResourceOwnerNewParent(portal->resowner, parentXactOwner); } diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 5e3b31ba90..1c960300fd 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -733,10 +733,25 @@ FreeSnapshot(Snapshot snapshot) */ void PushActiveSnapshot(Snapshot snap) +{ + PushActiveSnapshotWithLevel(snap, GetCurrentTransactionNestLevel()); +} + +/* + * PushActiveSnapshotWithLevel + * Set the given snapshot as the current active snapshot + * + * Same as PushActiveSnapshot except that caller can specify the + * transaction nesting level that "owns" the snapshot. This level + * must not be deeper than the current top of the snapshot stack. + */ +void +PushActiveSnapshotWithLevel(Snapshot snap, int snap_level) { ActiveSnapshotElt *newactive; Assert(snap != InvalidSnapshot); + Assert(ActiveSnapshot == NULL || snap_level >= ActiveSnapshot->as_level); newactive = MemoryContextAlloc(TopTransactionContext, sizeof(ActiveSnapshotElt)); @@ -750,7 +765,7 @@ PushActiveSnapshot(Snapshot snap) newactive->as_snap = snap; newactive->as_next = ActiveSnapshot; - newactive->as_level = GetCurrentTransactionNestLevel(); + newactive->as_level = snap_level; newactive->as_snap->active_count++; diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h index cb40bfa761..af0afcf985 100644 --- a/src/include/utils/portal.h +++ b/src/include/utils/portal.h @@ -195,6 +195,8 @@ typedef struct PortalData TimestampTz creation_time; /* time at which this portal was defined */ bool visible; /* include this portal in pg_cursors? */ + /* Stuff added at the end to avoid ABI break in stable branches: */ + /* * Outermost ActiveSnapshot for execution of the portal's queries. For * all but a few utility commands, we require such a snapshot to exist. @@ -202,6 +204,7 @@ typedef struct PortalData * and helps to reduce thrashing of the process's exposed xmin. */ Snapshot portalSnapshot; /* active snapshot, or NULL if none */ + int createLevel; /* creating subxact's nesting level */ } PortalData; /* @@ -219,6 +222,7 @@ extern void AtCleanup_Portals(void); extern void PortalErrorCleanup(void); extern void AtSubCommit_Portals(SubTransactionId mySubid, SubTransactionId parentSubid, + int parentLevel, ResourceOwner parentXactOwner); extern void AtSubAbort_Portals(SubTransactionId mySubid, SubTransactionId parentSubid, diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index ad7c15dc1a..651ff60998 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -110,6 +110,7 @@ extern void InvalidateCatalogSnapshot(void); extern void InvalidateCatalogSnapshotConditionally(void); extern void PushActiveSnapshot(Snapshot snapshot); +extern void PushActiveSnapshotWithLevel(Snapshot snapshot, int snap_level); extern void PushCopiedSnapshot(Snapshot snapshot); extern void UpdateActiveSnapshotCommandId(void); extern void PopActiveSnapshot(void); diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out index ab49db3f34..09a7e311e6 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out +++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out @@ -430,6 +430,34 @@ SELECT * FROM test1; ---+--- (0 rows) +-- test commit/rollback inside exception handler, too +TRUNCATE test1; +DO LANGUAGE plpgsql $$ +BEGIN + FOR i IN 1..10 LOOP + BEGIN + INSERT INTO test1 VALUES (i, 'good'); + INSERT INTO test1 VALUES (i/0, 'bad'); + EXCEPTION + WHEN division_by_zero THEN + INSERT INTO test1 VALUES (i, 'exception'); + IF (i % 3) > 0 THEN COMMIT; ELSE ROLLBACK; END IF; + END; + END LOOP; +END; +$$; +SELECT * FROM test1; + a | b +----+----------- + 1 | exception + 2 | exception + 4 | exception + 5 | exception + 7 | exception + 8 | exception + 10 | exception +(7 rows) + -- detoast result of simple expression after commit CREATE TEMP TABLE test4(f1 text); ALTER TABLE test4 ALTER COLUMN f1 SET STORAGE EXTERNAL; -- disable compression diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql index 888ddccace..8d76d00daa 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql @@ -354,6 +354,27 @@ $$; SELECT * FROM test1; +-- test commit/rollback inside exception handler, too +TRUNCATE test1; + +DO LANGUAGE plpgsql $$ +BEGIN + FOR i IN 1..10 LOOP + BEGIN + INSERT INTO test1 VALUES (i, 'good'); + INSERT INTO test1 VALUES (i/0, 'bad'); + EXCEPTION + WHEN division_by_zero THEN + INSERT INTO test1 VALUES (i, 'exception'); + IF (i % 3) > 0 THEN COMMIT; ELSE ROLLBACK; END IF; + END; + END LOOP; +END; +$$; + +SELECT * FROM test1; + + -- detoast result of simple expression after commit CREATE TEMP TABLE test4(f1 text); ALTER TABLE test4 ALTER COLUMN f1 SET STORAGE EXTERNAL; -- disable compression