Re: explicit tracking of ActiveSnapshot

[Alvaro Herrera <alvherre(at)commandprompt(dot)com>]

Alvaro Herrera wrote:

> In the previous installment,
> http://archives.postgresql.org/message-id/20080328140606(dot)GL7464(at)alvh(dot)no-ip(dot)org
> I was wondering whether the tracking of snapshots could be made more
> robust by having ActiveSnapshot be some sort of stack instead of a
> global pointer.  So I set to do that, and it turns out to appear a sane
> thing to do.  Here is a patch.

Here is the combined patch, keeping track of both ActiveSnapshot as a
stack and snapshots registered on a list.  I figured out that the
easiest way to manage the memory is to keep track of reference counts in
the snapshot itself, separately for the active snapshot stack and the
registered list, so we know the earliest time to free it.

Open issues:

- SerializableSnapshot is not handled.  (I think this is just a matter
of adding a RegisterSnapshot call as soon as the serializable snapshot
is created).

- Creating the VacuumXmin stuff to actually use it, and decide what to
do with GetOldestXmin.


Comments are very welcome.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.262
diff -c -p -r1.262 xact.c
*** src/backend/access/transam/xact.c	26 Mar 2008 18:48:59 -0000	1.262
--- src/backend/access/transam/xact.c	11 Apr 2008 17:07:51 -0000
*************** CommitTransaction(void)
*** 1752,1757 ****
--- 1752,1758 ----
  	AtEOXact_ComboCid();
  	AtEOXact_HashTables(true);
  	AtEOXact_PgStat(true);
+ 	AtEOXact_Snapshot(true);
  	pgstat_report_xact_timestamp(0);
  
  	CurrentResourceOwner = NULL;
*************** PrepareTransaction(void)
*** 1984,1989 ****
--- 1985,1991 ----
  	AtEOXact_ComboCid();
  	AtEOXact_HashTables(true);
  	/* don't call AtEOXact_PgStat here */
+ 	AtEOXact_Snapshot(true);
  
  	CurrentResourceOwner = NULL;
  	ResourceOwnerDelete(TopTransactionResourceOwner);
*************** AbortTransaction(void)
*** 2128,2133 ****
--- 2130,2136 ----
  	AtEOXact_ComboCid();
  	AtEOXact_HashTables(false);
  	AtEOXact_PgStat(false);
+ 	AtEOXact_Snapshot(false);
  	pgstat_report_xact_timestamp(0);
  
  	/*
*************** CommitSubTransaction(void)
*** 3806,3811 ****
--- 3809,3815 ----
  	AtSubCommit_Notify();
  	AtEOSubXact_UpdateFlatFiles(true, s->subTransactionId,
  								s->parent->subTransactionId);
+ 	AtSubCommit_Snapshot();
  
  	CallSubXactCallbacks(SUBXACT_EVENT_COMMIT_SUB, s->subTransactionId,
  						 s->parent->subTransactionId);
*************** AbortSubTransaction(void)
*** 3926,3931 ****
--- 3930,3936 ----
  		AtSubAbort_Notify();
  		AtEOSubXact_UpdateFlatFiles(false, s->subTransactionId,
  									s->parent->subTransactionId);
+ 		AtSubAbort_Snapshot();
  
  		/* Advertise the fact that we aborted in pg_clog. */
  		(void) RecordTransactionAbort(true);
Index: src/backend/catalog/index.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/catalog/index.c,v
retrieving revision 1.296
diff -c -p -r1.296 index.c
*** src/backend/catalog/index.c	26 Mar 2008 21:10:37 -0000	1.296
--- src/backend/catalog/index.c	11 Apr 2008 17:08:49 -0000
*************** IndexBuildHeapScan(Relation heapRelation
*** 1466,1471 ****
--- 1466,1472 ----
  	TransactionId OldestXmin;
  	BlockNumber root_blkno = InvalidBlockNumber;
  	OffsetNumber root_offsets[MaxHeapTuplesPerPage];
+ 	bool		unreg_snapshot = false;
  
  	/*
  	 * sanity checks
*************** IndexBuildHeapScan(Relation heapRelation
*** 1502,1508 ****
  	}
  	else if (indexInfo->ii_Concurrent)
  	{
! 		snapshot = CopySnapshot(GetTransactionSnapshot());
  		OldestXmin = InvalidTransactionId;		/* not used */
  	}
  	else
--- 1503,1510 ----
  	}
  	else if (indexInfo->ii_Concurrent)
  	{
! 		snapshot = RegisterSnapshot(GetTransactionSnapshot());
! 		unreg_snapshot = true;
  		OldestXmin = InvalidTransactionId;		/* not used */
  	}
  	else
*************** IndexBuildHeapScan(Relation heapRelation
*** 1787,1792 ****
--- 1789,1796 ----
  	}
  
  	heap_endscan(scan);
+ 	if (unreg_snapshot)
+ 		UnregisterSnapshot(snapshot);
  
  	ExecDropSingleTupleTableSlot(slot);
  
Index: src/backend/commands/cluster.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/cluster.c,v
retrieving revision 1.173
diff -c -p -r1.173 cluster.c
*** src/backend/commands/cluster.c	13 Apr 2008 19:18:14 -0000	1.173
--- src/backend/commands/cluster.c	14 Apr 2008 13:21:14 -0000
*************** cluster(ClusterStmt *stmt, bool isTopLev
*** 211,216 ****
--- 211,217 ----
  		rvs = get_tables_to_cluster(cluster_context);
  
  		/* Commit to get out of starting transaction */
+ 		PopActiveSnapshot();
  		CommitTransactionCommand();
  
  		/* Ok, now that we've got them all, cluster them one by one */
*************** cluster(ClusterStmt *stmt, bool isTopLev
*** 221,228 ****
  			/* Start a new transaction for each relation. */
  			StartTransactionCommand();
  			/* functions in indexes may want a snapshot set */
! 			ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  			cluster_rel(rvtc, true);
  			CommitTransactionCommand();
  		}
  
--- 222,230 ----
  			/* Start a new transaction for each relation. */
  			StartTransactionCommand();
  			/* functions in indexes may want a snapshot set */
! 			PushActiveSnapshot(GetTransactionSnapshot());
  			cluster_rel(rvtc, true);
+ 			PopActiveSnapshot();
  			CommitTransactionCommand();
  		}
  
Index: src/backend/commands/copy.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.298
diff -c -p -r1.298 copy.c
*** src/backend/commands/copy.c	26 Mar 2008 18:48:59 -0000	1.298
--- src/backend/commands/copy.c	14 Apr 2008 17:36:17 -0000
*************** DoCopy(const CopyStmt *stmt, const char 
*** 1003,1008 ****
--- 1003,1009 ----
  		Query	   *query;
  		PlannedStmt *plan;
  		DestReceiver *dest;
+ 		Snapshot	snap;
  
  		Assert(!is_from);
  		cstate->rel = NULL;
*************** DoCopy(const CopyStmt *stmt, const char 
*** 1044,1064 ****
  		plan = planner(query, 0, NULL);
  
  		/*
! 		 * Update snapshot command ID to ensure this query sees results of any
! 		 * previously executed queries.  (It's a bit cheesy to modify
! 		 * ActiveSnapshot without making a copy, but for the limited ways in
! 		 * which COPY can be invoked, I think it's OK, because the active
! 		 * snapshot shouldn't be shared with anything else anyway.)
  		 */
! 		ActiveSnapshot->curcid = GetCurrentCommandId(false);
  
  		/* Create dest receiver for COPY OUT */
  		dest = CreateDestReceiver(DestCopyOut, NULL);
  		((DR_copy *) dest)->cstate = cstate;
  
  		/* Create a QueryDesc requesting no output */
! 		cstate->queryDesc = CreateQueryDesc(plan,
! 											ActiveSnapshot, InvalidSnapshot,
  											dest, NULL, false);
  
  		/*
--- 1045,1063 ----
  		plan = planner(query, 0, NULL);
  
  		/*
! 		 * Use a snapshot with an updated command ID to ensure this query sees
! 		 * results of any previously executed queries.  We cannot modify the
! 		 * active snapshot in place, so get a copy instead.
  		 */
! 		snap = CopySnapshot(GetActiveSnapshot());
! 		snap->curcid = GetCurrentCommandId(false);
  
  		/* Create dest receiver for COPY OUT */
  		dest = CreateDestReceiver(DestCopyOut, NULL);
  		((DR_copy *) dest)->cstate = cstate;
  
  		/* Create a QueryDesc requesting no output */
! 		cstate->queryDesc = CreateQueryDesc(plan, snap, InvalidSnapshot,
  											dest, NULL, false);
  
  		/*
*************** CopyTo(CopyState cstate)
*** 1390,1396 ****
  		values = (Datum *) palloc(num_phys_attrs * sizeof(Datum));
  		nulls = (bool *) palloc(num_phys_attrs * sizeof(bool));
  
! 		scandesc = heap_beginscan(cstate->rel, ActiveSnapshot, 0, NULL);
  
  		while ((tuple = heap_getnext(scandesc, ForwardScanDirection)) != NULL)
  		{
--- 1389,1395 ----
  		values = (Datum *) palloc(num_phys_attrs * sizeof(Datum));
  		nulls = (bool *) palloc(num_phys_attrs * sizeof(bool));
  
! 		scandesc = heap_beginscan(cstate->rel, GetActiveSnapshot(), 0, NULL);
  
  		while ((tuple = heap_getnext(scandesc, ForwardScanDirection)) != NULL)
  		{
Index: src/backend/commands/explain.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/explain.c,v
retrieving revision 1.171
diff -c -p -r1.171 explain.c
*** src/backend/commands/explain.c	26 Mar 2008 18:48:59 -0000	1.171
--- src/backend/commands/explain.c	14 Apr 2008 17:36:42 -0000
*************** ExplainOnePlan(PlannedStmt *plannedstmt,
*** 226,244 ****
  	ExplainState *es;
  	StringInfoData buf;
  	int			eflags;
  
  	/*
! 	 * Update snapshot command ID to ensure this query sees results of any
! 	 * previously executed queries.  (It's a bit cheesy to modify
! 	 * ActiveSnapshot without making a copy, but for the limited ways in which
! 	 * EXPLAIN can be invoked, I think it's OK, because the active snapshot
! 	 * shouldn't be shared with anything else anyway.)
  	 */
! 	ActiveSnapshot->curcid = GetCurrentCommandId(false);
  
  	/* Create a QueryDesc requesting no output */
  	queryDesc = CreateQueryDesc(plannedstmt,
! 								ActiveSnapshot, InvalidSnapshot,
  								None_Receiver, params,
  								stmt->analyze);
  
--- 226,244 ----
  	ExplainState *es;
  	StringInfoData buf;
  	int			eflags;
+ 	Snapshot	snap;
  
  	/*
! 	 * Use a snapshot with an updates command ID to ensure this query sees
! 	 * results of any previously executed queries.  We cannot modify the
! 	 * active snapshot in place, so get a copy instead.
  	 */
! 	snap = CopySnapshot(GetActiveSnapshot());
! 	snap->curcid = GetCurrentCommandId(false);
  
  	/* Create a QueryDesc requesting no output */
  	queryDesc = CreateQueryDesc(plannedstmt,
! 								snap, InvalidSnapshot,
  								None_Receiver, params,
  								stmt->analyze);
  
Index: src/backend/commands/indexcmds.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/indexcmds.c,v
retrieving revision 1.174
diff -c -p -r1.174 indexcmds.c
*** src/backend/commands/indexcmds.c	26 Mar 2008 21:10:37 -0000	1.174
--- src/backend/commands/indexcmds.c	14 Apr 2008 23:46:23 -0000
*************** DefineIndex(RangeVar *heapRelation,
*** 483,488 ****
--- 483,489 ----
  	 */
  	LockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock);
  
+ 	PopActiveSnapshot();
  	CommitTransactionCommand();
  	StartTransactionCommand();
  
*************** DefineIndex(RangeVar *heapRelation,
*** 541,547 ****
  	indexRelation = index_open(indexRelationId, RowExclusiveLock);
  
  	/* Set ActiveSnapshot since functions in the indexes may need it */
! 	ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  
  	/* We have to re-build the IndexInfo struct, since it was lost in commit */
  	indexInfo = BuildIndexInfo(indexRelation);
--- 542,548 ----
  	indexRelation = index_open(indexRelationId, RowExclusiveLock);
  
  	/* Set ActiveSnapshot since functions in the indexes may need it */
! 	PushActiveSnapshot(GetTransactionSnapshot());
  
  	/* We have to re-build the IndexInfo struct, since it was lost in commit */
  	indexInfo = BuildIndexInfo(indexRelation);
*************** DefineIndex(RangeVar *heapRelation,
*** 580,585 ****
--- 581,589 ----
  
  	heap_close(pg_index, RowExclusiveLock);
  
+ 	/* we can do away with our snapshot */
+ 	PopActiveSnapshot();
+ 
  	/*
  	 * Commit this transaction to make the indisready update visible.
  	 */
*************** DefineIndex(RangeVar *heapRelation,
*** 615,628 ****
  	 * We also set ActiveSnapshot to this snap, since functions in indexes may
  	 * need a snapshot.
  	 */
! 	snapshot = CopySnapshot(GetTransactionSnapshot());
! 	ActiveSnapshot = snapshot;
  
  	/*
  	 * Scan the index and the heap, insert any missing index entries.
  	 */
  	validate_index(relationId, indexRelationId, snapshot);
  
  	/*
  	 * The index is now valid in the sense that it contains all currently
  	 * interesting tuples.	But since it might not contain tuples deleted just
--- 619,635 ----
  	 * We also set ActiveSnapshot to this snap, since functions in indexes may
  	 * need a snapshot.
  	 */
! 	snapshot = RegisterSnapshot(GetTransactionSnapshot());
! 	PushActiveSnapshot(snapshot);
  
  	/*
  	 * Scan the index and the heap, insert any missing index entries.
  	 */
  	validate_index(relationId, indexRelationId, snapshot);
  
+ 	/* We no longer need the validating snapshot here anymore */
+ 	UnregisterSnapshot(snapshot);
+ 	
  	/*
  	 * The index is now valid in the sense that it contains all currently
  	 * interesting tuples.	But since it might not contain tuples deleted just
*************** DefineIndex(RangeVar *heapRelation,
*** 644,650 ****
  	 * Also, GetCurrentVirtualXIDs never reports our own vxid, so we need not
  	 * check for that.
  	 */
! 	old_snapshots = GetCurrentVirtualXIDs(ActiveSnapshot->xmax, false,
  										  PROC_IS_AUTOVACUUM | PROC_IN_VACUUM);
  
  	while (VirtualTransactionIdIsValid(*old_snapshots))
--- 651,657 ----
  	 * Also, GetCurrentVirtualXIDs never reports our own vxid, so we need not
  	 * check for that.
  	 */
! 	old_snapshots = GetCurrentVirtualXIDs(snapshot->xmax, false,
  										  PROC_IS_AUTOVACUUM | PROC_IN_VACUUM);
  
  	while (VirtualTransactionIdIsValid(*old_snapshots))
*************** DefineIndex(RangeVar *heapRelation,
*** 675,680 ****
--- 682,688 ----
  
  	heap_close(pg_index, RowExclusiveLock);
  
+ 
  	/*
  	 * The pg_index update will cause backends (including this one) to update
  	 * relcache entries for the index itself, but we should also send a
*************** DefineIndex(RangeVar *heapRelation,
*** 685,690 ****
--- 693,701 ----
  	 */
  	CacheInvalidateRelcacheByRelid(heaprelid.relId);
  
+ 	/* we can now do away with our active snapshot */
+ 	PopActiveSnapshot();
+ 
  	/*
  	 * Last thing to do is release the session-level lock on the parent table.
  	 */
*************** ReindexDatabase(const char *databaseName
*** 1452,1457 ****
--- 1463,1469 ----
  	heap_close(relationRelation, AccessShareLock);
  
  	/* Now reindex each rel in a separate transaction */
+ 	PopActiveSnapshot();
  	CommitTransactionCommand();
  	foreach(l, relids)
  	{
*************** ReindexDatabase(const char *databaseName
*** 1459,1469 ****
  
  		StartTransactionCommand();
  		/* functions in indexes may want a snapshot set */
! 		ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  		if (reindex_relation(relid, true))
  			ereport(NOTICE,
  					(errmsg("table \"%s\" was reindexed",
  							get_rel_name(relid))));
  		CommitTransactionCommand();
  	}
  	StartTransactionCommand();
--- 1471,1482 ----
  
  		StartTransactionCommand();
  		/* functions in indexes may want a snapshot set */
! 		PushActiveSnapshot(GetTransactionSnapshot());
  		if (reindex_relation(relid, true))
  			ereport(NOTICE,
  					(errmsg("table \"%s\" was reindexed",
  							get_rel_name(relid))));
+ 		PopActiveSnapshot();
  		CommitTransactionCommand();
  	}
  	StartTransactionCommand();
Index: src/backend/commands/portalcmds.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/portalcmds.c,v
retrieving revision 1.73
diff -c -p -r1.73 portalcmds.c
*** src/backend/commands/portalcmds.c	2 Apr 2008 18:31:50 -0000	1.73
--- src/backend/commands/portalcmds.c	11 Apr 2008 17:07:51 -0000
*************** PerformCursorOpen(PlannedStmt *stmt, Par
*** 121,127 ****
  	/*
  	 * Start execution, inserting parameters if any.
  	 */
! 	PortalStart(portal, params, ActiveSnapshot);
  
  	Assert(portal->strategy == PORTAL_ONE_SELECT);
  
--- 121,127 ----
  	/*
  	 * Start execution, inserting parameters if any.
  	 */
! 	PortalStart(portal, params, GetActiveSnapshot());
  
  	Assert(portal->strategy == PORTAL_ONE_SELECT);
  
*************** PersistHoldablePortal(Portal portal)
*** 293,299 ****
  {
  	QueryDesc  *queryDesc = PortalGetQueryDesc(portal);
  	Portal		saveActivePortal;
- 	Snapshot	saveActiveSnapshot;
  	ResourceOwner saveResourceOwner;
  	MemoryContext savePortalContext;
  	MemoryContext oldcxt;
--- 293,298 ----
*************** PersistHoldablePortal(Portal portal)
*** 334,351 ****
  	 * Set up global portal context pointers.
  	 */
  	saveActivePortal = ActivePortal;
- 	saveActiveSnapshot = ActiveSnapshot;
  	saveResourceOwner = CurrentResourceOwner;
  	savePortalContext = PortalContext;
  	PG_TRY();
  	{
  		ActivePortal = portal;
- 		ActiveSnapshot = queryDesc->snapshot;
  		CurrentResourceOwner = portal->resowner;
  		PortalContext = PortalGetHeapMemory(portal);
  
  		MemoryContextSwitchTo(PortalContext);
  
  		/*
  		 * Rewind the executor: we need to store the entire result set in the
  		 * tuplestore, so that subsequent backward FETCHs can be processed.
--- 333,350 ----
  	 * Set up global portal context pointers.
  	 */
  	saveActivePortal = ActivePortal;
  	saveResourceOwner = CurrentResourceOwner;
  	savePortalContext = PortalContext;
  	PG_TRY();
  	{
  		ActivePortal = portal;
  		CurrentResourceOwner = portal->resowner;
  		PortalContext = PortalGetHeapMemory(portal);
  
  		MemoryContextSwitchTo(PortalContext);
  
+ 		PushActiveSnapshot(queryDesc->snapshot);
+ 
  		/*
  		 * Rewind the executor: we need to store the entire result set in the
  		 * tuplestore, so that subsequent backward FETCHs can be processed.
*************** PersistHoldablePortal(Portal portal)
*** 411,417 ****
  
  		/* Restore global vars and propagate error */
  		ActivePortal = saveActivePortal;
- 		ActiveSnapshot = saveActiveSnapshot;
  		CurrentResourceOwner = saveResourceOwner;
  		PortalContext = savePortalContext;
  
--- 410,415 ----
*************** PersistHoldablePortal(Portal portal)
*** 425,434 ****
  	portal->status = PORTAL_READY;
  
  	ActivePortal = saveActivePortal;
- 	ActiveSnapshot = saveActiveSnapshot;
  	CurrentResourceOwner = saveResourceOwner;
  	PortalContext = savePortalContext;
  
  	/*
  	 * We can now release any subsidiary memory of the portal's heap context;
  	 * we'll never use it again.  The executor already dropped its context,
--- 423,433 ----
  	portal->status = PORTAL_READY;
  
  	ActivePortal = saveActivePortal;
  	CurrentResourceOwner = saveResourceOwner;
  	PortalContext = savePortalContext;
  
+ 	PopActiveSnapshot();
+ 
  	/*
  	 * We can now release any subsidiary memory of the portal's heap context;
  	 * we'll never use it again.  The executor already dropped its context,
Index: src/backend/commands/prepare.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/prepare.c,v
retrieving revision 1.85
diff -c -p -r1.85 prepare.c
*** src/backend/commands/prepare.c	2 Apr 2008 18:31:50 -0000	1.85
--- src/backend/commands/prepare.c	11 Apr 2008 17:07:51 -0000
*************** ExecuteQuery(ExecuteStmt *stmt, const ch
*** 264,270 ****
  	/*
  	 * Run the portal to completion.
  	 */
! 	PortalStart(portal, paramLI, ActiveSnapshot);
  
  	(void) PortalRun(portal, FETCH_ALL, false, dest, dest, completionTag);
  
--- 264,270 ----
  	/*
  	 * Run the portal to completion.
  	 */
! 	PortalStart(portal, paramLI, GetActiveSnapshot());
  
  	(void) PortalRun(portal, FETCH_ALL, false, dest, dest, completionTag);
  
Index: src/backend/commands/trigger.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/trigger.c,v
retrieving revision 1.231
diff -c -p -r1.231 trigger.c
*** src/backend/commands/trigger.c	28 Mar 2008 00:21:55 -0000	1.231
--- src/backend/commands/trigger.c	11 Apr 2008 18:59:17 -0000
*************** void
*** 2974,2979 ****
--- 2974,2980 ----
  AfterTriggerFireDeferred(void)
  {
  	AfterTriggerEventList *events;
+ 	bool		snap_pushed = false;
  
  	/* Must be inside a transaction */
  	Assert(afterTriggers != NULL);
*************** AfterTriggerFireDeferred(void)
*** 2988,2994 ****
  	 */
  	events = &afterTriggers->events;
  	if (events->head != NULL)
! 		ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  
  	/*
  	 * Run all the remaining triggers.	Loop until they are all gone, in case
--- 2989,2998 ----
  	 */
  	events = &afterTriggers->events;
  	if (events->head != NULL)
! 	{
! 		PushActiveSnapshot(GetTransactionSnapshot());
! 		snap_pushed = true;
! 	}
  
  	/*
  	 * Run all the remaining triggers.	Loop until they are all gone, in case
*************** AfterTriggerFireDeferred(void)
*** 3001,3006 ****
--- 3005,3013 ----
  		afterTriggerInvokeEvents(events, firing_id, NULL, true);
  	}
  
+ 	if (snap_pushed)
+ 		PopActiveSnapshot();
+ 
  	Assert(events->head == NULL);
  }
  
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.371
diff -c -p -r1.371 vacuum.c
*** src/backend/commands/vacuum.c	26 Mar 2008 21:10:38 -0000	1.371
--- src/backend/commands/vacuum.c	11 Apr 2008 18:56:58 -0000
*************** vacuum(VacuumStmt *vacstmt, List *relids
*** 407,412 ****
--- 407,416 ----
  	 */
  	if (use_own_xacts)
  	{
+ 		/* ActiveSnapshot is not set by autovacuum */
+ 		if (ActiveSnapshotSet())
+ 			PopActiveSnapshot();
+ 
  		/* matches the StartTransaction in PostgresMain() */
  		CommitTransactionCommand();
  	}
*************** vacuum(VacuumStmt *vacstmt, List *relids
*** 444,450 ****
  				{
  					StartTransactionCommand();
  					/* functions in indexes may want a snapshot set */
! 					ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  				}
  				else
  					old_context = MemoryContextSwitchTo(anl_context);
--- 448,454 ----
  				{
  					StartTransactionCommand();
  					/* functions in indexes may want a snapshot set */
! 					PushActiveSnapshot(GetTransactionSnapshot());
  				}
  				else
  					old_context = MemoryContextSwitchTo(anl_context);
*************** vacuum(VacuumStmt *vacstmt, List *relids
*** 452,458 ****
--- 456,465 ----
  				analyze_rel(relid, vacstmt, vac_strategy);
  
  				if (use_own_xacts)
+ 				{
+ 					PopActiveSnapshot();
  					CommitTransactionCommand();
+ 				}
  				else
  				{
  					MemoryContextSwitchTo(old_context);
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 979,985 ****
  	if (vacstmt->full)
  	{
  		/* functions in indexes may want a snapshot set */
! 		ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  	}
  	else
  	{
--- 986,992 ----
  	if (vacstmt->full)
  	{
  		/* functions in indexes may want a snapshot set */
! 		PushActiveSnapshot(GetTransactionSnapshot());
  	}
  	else
  	{
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1036,1041 ****
--- 1043,1050 ----
  
  	if (!onerel)
  	{
+ 		if (vacstmt->full)
+ 			PopActiveSnapshot();
  		CommitTransactionCommand();
  		return;
  	}
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1066,1071 ****
--- 1075,1082 ----
  					(errmsg("skipping \"%s\" --- only table or database owner can vacuum it",
  							RelationGetRelationName(onerel))));
  		relation_close(onerel, lmode);
+ 		if (vacstmt->full)
+ 			PopActiveSnapshot();
  		CommitTransactionCommand();
  		return;
  	}
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1080,1085 ****
--- 1091,1098 ----
  				(errmsg("skipping \"%s\" --- cannot vacuum indexes, views, or special system tables",
  						RelationGetRelationName(onerel))));
  		relation_close(onerel, lmode);
+ 		if (vacstmt->full)
+ 			PopActiveSnapshot();
  		CommitTransactionCommand();
  		return;
  	}
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1094,1099 ****
--- 1107,1114 ----
  	if (isOtherTempNamespace(RelationGetNamespace(onerel)))
  	{
  		relation_close(onerel, lmode);
+ 		if (vacstmt->full)
+ 			PopActiveSnapshot();
  		CommitTransactionCommand();
  		return;
  	}
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1141,1146 ****
--- 1156,1163 ----
  	/*
  	 * Complete the transaction and free all temporary memory used.
  	 */
+ 	if (vacstmt->full)
+ 		PopActiveSnapshot();
  	CommitTransactionCommand();
  
  	/*
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.305
diff -c -p -r1.305 execMain.c
*** src/backend/executor/execMain.c	28 Mar 2008 00:21:55 -0000	1.305
--- src/backend/executor/execMain.c	11 Apr 2008 17:08:54 -0000
***************
*** 52,57 ****
--- 52,58 ----
  #include "utils/acl.h"
  #include "utils/lsyscache.h"
  #include "utils/memutils.h"
+ #include "utils/snapmgr.h"
  #include "utils/tqual.h"
  
  
*************** ExecutorStart(QueryDesc *queryDesc, int 
*** 183,190 ****
  	/*
  	 * Copy other important information into the EState
  	 */
! 	estate->es_snapshot = queryDesc->snapshot;
! 	estate->es_crosscheck_snapshot = queryDesc->crosscheck_snapshot;
  	estate->es_instrument = queryDesc->doInstrument;
  
  	/*
--- 184,191 ----
  	/*
  	 * Copy other important information into the EState
  	 */
! 	estate->es_snapshot = RegisterSnapshot(queryDesc->snapshot);
! 	estate->es_crosscheck_snapshot = RegisterSnapshot(queryDesc->crosscheck_snapshot);
  	estate->es_instrument = queryDesc->doInstrument;
  
  	/*
*************** ExecutorEnd(QueryDesc *queryDesc)
*** 311,316 ****
--- 312,321 ----
  	if (estate->es_select_into)
  		CloseIntoRel(queryDesc);
  
+ 	/* do away with our snapshots */
+ 	UnregisterSnapshot(estate->es_snapshot);
+ 	UnregisterSnapshot(estate->es_crosscheck_snapshot);
+ 
  	/*
  	 * Must switch out of context before destroying it
  	 */
Index: src/backend/executor/functions.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/executor/functions.c,v
retrieving revision 1.124
diff -c -p -r1.124 functions.c
*** src/backend/executor/functions.c	26 Mar 2008 18:48:59 -0000	1.124
--- src/backend/executor/functions.c	11 Apr 2008 18:33:40 -0000
*************** postquel_start(execution_state *es, SQLF
*** 295,309 ****
  	 * In a read-only function, use the surrounding query's snapshot;
  	 * otherwise take a new snapshot for each query.  The snapshot should
  	 * include a fresh command ID so that all work to date in this transaction
! 	 * is visible.	We copy in both cases so that postquel_end can
! 	 * unconditionally do FreeSnapshot.
  	 */
  	if (fcache->readonly_func)
! 		snapshot = CopySnapshot(ActiveSnapshot);
  	else
  	{
  		CommandCounterIncrement();
! 		snapshot = CopySnapshot(GetTransactionSnapshot());
  	}
  
  	if (IsA(es->stmt, PlannedStmt))
--- 295,308 ----
  	 * In a read-only function, use the surrounding query's snapshot;
  	 * otherwise take a new snapshot for each query.  The snapshot should
  	 * include a fresh command ID so that all work to date in this transaction
! 	 * is visible.
  	 */
  	if (fcache->readonly_func)
! 		snapshot = GetActiveSnapshot();
  	else
  	{
  		CommandCounterIncrement();
! 		snapshot = GetTransactionSnapshot();
  	}
  
  	if (IsA(es->stmt, PlannedStmt))
*************** static TupleTableSlot *
*** 340,353 ****
  postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache)
  {
  	TupleTableSlot *result;
- 	Snapshot	saveActiveSnapshot;
  	long		count;
  
  	/* Make our snapshot the active one for any called functions */
! 	saveActiveSnapshot = ActiveSnapshot;
! 	PG_TRY();
! 	{
! 		ActiveSnapshot = es->qd->snapshot;
  
  		if (es->qd->utilitystmt)
  		{
--- 339,348 ----
  postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache)
  {
  	TupleTableSlot *result;
  	long		count;
  
  	/* Make our snapshot the active one for any called functions */
! 	PushActiveSnapshot(es->qd->snapshot);
  
  		if (es->qd->utilitystmt)
  		{
*************** postquel_getnext(execution_state *es, SQ
*** 380,395 ****
  
  			result = ExecutorRun(es->qd, ForwardScanDirection, count);
  		}
- 	}
- 	PG_CATCH();
- 	{
- 		/* Restore global vars and propagate error */
- 		ActiveSnapshot = saveActiveSnapshot;
- 		PG_RE_THROW();
- 	}
- 	PG_END_TRY();
  
! 	ActiveSnapshot = saveActiveSnapshot;
  
  	return result;
  }
--- 375,382 ----
  
  			result = ExecutorRun(es->qd, ForwardScanDirection, count);
  		}
  
! 	PopActiveSnapshot();
  
  	return result;
  }
*************** postquel_getnext(execution_state *es, SQ
*** 397,404 ****
  static void
  postquel_end(execution_state *es)
  {
- 	Snapshot	saveActiveSnapshot;
- 
  	/* mark status done to ensure we don't do ExecutorEnd twice */
  	es->status = F_EXEC_DONE;
  
--- 384,389 ----
*************** postquel_end(execution_state *es)
*** 406,431 ****
  	if (es->qd->utilitystmt == NULL)
  	{
  		/* Make our snapshot the active one for any called functions */
! 		saveActiveSnapshot = ActiveSnapshot;
! 		PG_TRY();
! 		{
! 			ActiveSnapshot = es->qd->snapshot;
  
  			if (es->qd->operation != CMD_SELECT)
  				AfterTriggerEndQuery(es->qd->estate);
  			ExecutorEnd(es->qd);
! 		}
! 		PG_CATCH();
! 		{
! 			/* Restore global vars and propagate error */
! 			ActiveSnapshot = saveActiveSnapshot;
! 			PG_RE_THROW();
! 		}
! 		PG_END_TRY();
! 		ActiveSnapshot = saveActiveSnapshot;
  	}
  
- 	FreeSnapshot(es->qd->snapshot);
  	FreeQueryDesc(es->qd);
  	es->qd = NULL;
  }
--- 391,405 ----
  	if (es->qd->utilitystmt == NULL)
  	{
  		/* Make our snapshot the active one for any called functions */
! 		PushActiveSnapshot(es->qd->snapshot);
  
  			if (es->qd->operation != CMD_SELECT)
  				AfterTriggerEndQuery(es->qd->estate);
  			ExecutorEnd(es->qd);
! 
! 		PopActiveSnapshot();
  	}
  
  	FreeQueryDesc(es->qd);
  	es->qd = NULL;
  }
Index: src/backend/executor/spi.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/executor/spi.c,v
retrieving revision 1.193
diff -c -p -r1.193 spi.c
*** src/backend/executor/spi.c	2 Apr 2008 18:31:50 -0000	1.193
--- src/backend/executor/spi.c	14 Apr 2008 17:37:50 -0000
*************** SPI_execp(SPIPlanPtr plan, Datum *Values
*** 371,379 ****
  
  /*
   * SPI_execute_snapshot -- identical to SPI_execute_plan, except that we allow
!  * the caller to specify exactly which snapshots to use.  Also, the caller
!  * may specify that AFTER triggers should be queued as part of the outer
!  * query rather than being fired immediately at the end of the command.
   *
   * This is currently not documented in spi.sgml because it is only intended
   * for use by RI triggers.
--- 371,380 ----
  
  /*
   * SPI_execute_snapshot -- identical to SPI_execute_plan, except that we allow
!  * the caller to specify exactly which snapshots to use, which will be
!  * registered here.  Also, the caller may specify that AFTER triggers should be
!  * queued as part of the outer query rather than being fired immediately at the
!  * end of the command.
   *
   * This is currently not documented in spi.sgml because it is only intended
   * for use by RI triggers.
*************** SPI_cursor_open(const char *name, SPIPla
*** 1101,1111 ****
  	}
  
  	/*
! 	 * Set up the snapshot to use.	(PortalStart will do CopySnapshot, so we
  	 * skip that here.)
  	 */
  	if (read_only)
! 		snapshot = ActiveSnapshot;
  	else
  	{
  		CommandCounterIncrement();
--- 1102,1112 ----
  	}
  
  	/*
! 	 * Set up the snapshot to use.	(PortalStart will do RegisterSnapshot, so we
  	 * skip that here.)
  	 */
  	if (read_only)
! 		snapshot = GetActiveSnapshot();
  	else
  	{
  		CommandCounterIncrement();
*************** _SPI_execute_plan(SPIPlanPtr plan, Param
*** 1609,1623 ****
  	volatile Oid my_lastoid = InvalidOid;
  	SPITupleTable *volatile my_tuptable = NULL;
  	volatile int res = 0;
! 	Snapshot	saveActiveSnapshot;
! 
! 	/* Be sure to restore ActiveSnapshot on error exit */
! 	saveActiveSnapshot = ActiveSnapshot;
! 	PG_TRY();
! 	{
! 		ErrorContextCallback spierrcontext;
! 		CachedPlan *cplan = NULL;
! 		ListCell   *lc1;
  
  		/*
  		 * Setup error traceback support for ereport()
--- 1610,1619 ----
  	volatile Oid my_lastoid = InvalidOid;
  	SPITupleTable *volatile my_tuptable = NULL;
  	volatile int res = 0;
! 	bool		have_active_snap = ActiveSnapshotSet();
! 	ErrorContextCallback spierrcontext;
! 	CachedPlan *cplan = NULL;
! 	ListCell   *lc1;
  
  		/*
  		 * Setup error traceback support for ereport()
*************** _SPI_execute_plan(SPIPlanPtr plan, Param
*** 1653,1658 ****
--- 1649,1655 ----
  				Node	   *stmt = (Node *) lfirst(lc2);
  				bool		canSetTag;
  				DestReceiver *dest;
+ 				bool		pushed_active_snap = false;
  
  				_SPI_current->processed = 0;
  				_SPI_current->lastoid = InvalidOid;
*************** _SPI_execute_plan(SPIPlanPtr plan, Param
*** 1708,1737 ****
  					 * ActiveSnapshot; if read-write, grab a full new snap.
  					 */
  					if (read_only)
! 						ActiveSnapshot = CopySnapshot(saveActiveSnapshot);
  					else
! 						ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  				}
  				else
  				{
  					/*
  					 * We interpret read_only with a specified snapshot to be
  					 * exactly that snapshot, but read-write means use the
! 					 * snap with advancing of command ID.
  					 */
! 					ActiveSnapshot = CopySnapshot(snapshot);
  					if (!read_only)
! 						ActiveSnapshot->curcid = GetCurrentCommandId(false);
  				}
  
  				if (IsA(stmt, PlannedStmt) &&
  					((PlannedStmt *) stmt)->utilityStmt == NULL)
  				{
  					QueryDesc  *qdesc;
  
  					qdesc = CreateQueryDesc((PlannedStmt *) stmt,
! 											ActiveSnapshot,
! 											crosscheck_snapshot,
  											dest,
  											paramLI, false);
  					res = _SPI_pquery(qdesc, fire_triggers,
--- 1705,1753 ----
  					 * ActiveSnapshot; if read-write, grab a full new snap.
  					 */
  					if (read_only)
! 					{
! 						if (have_active_snap)
! 						{
! 							PushActiveSnapshot(GetActiveSnapshot());
! 							pushed_active_snap = true;
! 						}
! 					}
  					else
! 					{
! 						PushActiveSnapshot(GetTransactionSnapshot());
! 						pushed_active_snap = true;
! 					}
  				}
  				else
  				{
+ 					Snapshot	snap;
+ 
  					/*
  					 * We interpret read_only with a specified snapshot to be
  					 * exactly that snapshot, but read-write means use the
! 					 * snap with advancing of command ID.  We cannot modify the
! 					 * active snapshot in place, so get a copy instead.
  					 */
! 					snap = CopySnapshot(snapshot);
  					if (!read_only)
! 						snap->curcid = GetCurrentCommandId(false);
! 					PushActiveSnapshot(snap);
! 					pushed_active_snap = true;
  				}
  
  				if (IsA(stmt, PlannedStmt) &&
  					((PlannedStmt *) stmt)->utilityStmt == NULL)
  				{
  					QueryDesc  *qdesc;
+ 					Snapshot	snap;
+ 
+ 					if (ActiveSnapshotSet())
+ 						snap = GetActiveSnapshot();
+ 					else
+ 						snap = InvalidSnapshot;
  
  					qdesc = CreateQueryDesc((PlannedStmt *) stmt,
! 											snap, crosscheck_snapshot,
  											dest,
  											paramLI, false);
  					res = _SPI_pquery(qdesc, fire_triggers,
*************** _SPI_execute_plan(SPIPlanPtr plan, Param
*** 1751,1758 ****
  						_SPI_current->processed = _SPI_current->tuptable->alloced - _SPI_current->tuptable->free;
  					res = SPI_OK_UTILITY;
  				}
! 				FreeSnapshot(ActiveSnapshot);
! 				ActiveSnapshot = NULL;
  
  				/*
  				 * The last canSetTag query sets the status values returned to
--- 1767,1775 ----
  						_SPI_current->processed = _SPI_current->tuptable->alloced - _SPI_current->tuptable->free;
  					res = SPI_OK_UTILITY;
  				}
! 
! 				if (pushed_active_snap)
! 					PopActiveSnapshot();
  
  				/*
  				 * The last canSetTag query sets the status values returned to
*************** fail:
*** 1804,1819 ****
  		 * Pop the error context stack
  		 */
  		error_context_stack = spierrcontext.previous;
- 	}
- 	PG_CATCH();
- 	{
- 		/* Restore global vars and propagate error */
- 		ActiveSnapshot = saveActiveSnapshot;
- 		PG_RE_THROW();
- 	}
- 	PG_END_TRY();
- 
- 	ActiveSnapshot = saveActiveSnapshot;
  
  	/* Save results for caller */
  	SPI_processed = my_processed;
--- 1821,1826 ----
Index: src/backend/storage/ipc/procarray.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/storage/ipc/procarray.c,v
retrieving revision 1.43
diff -c -p -r1.43 procarray.c
*** src/backend/storage/ipc/procarray.c	26 Mar 2008 18:48:59 -0000	1.43
--- src/backend/storage/ipc/procarray.c	14 Apr 2008 23:41:33 -0000
*************** GetSnapshotData(Snapshot snapshot, bool 
*** 681,691 ****
  
  	Assert(snapshot != NULL);
  
- 	/* Serializable snapshot must be computed before any other... */
- 	Assert(serializable ?
- 		   !TransactionIdIsValid(MyProc->xmin) :
- 		   TransactionIdIsValid(MyProc->xmin));
- 
  	/*
  	 * Allocating space for maxProcs xids is usually overkill; numProcs would
  	 * be sufficient.  But it seems better to do the malloc while not holding
--- 681,686 ----
*************** GetSnapshotData(Snapshot snapshot, bool 
*** 828,833 ****
--- 823,832 ----
  	snapshot->xcnt = count;
  	snapshot->subxcnt = subcount;
  
+ 	snapshot->active_count = 0;
+ 	snapshot->regd_count = 0;
+ 	snapshot->copied = false;
+ 
  	snapshot->curcid = GetCurrentCommandId(false);
  
  	return snapshot;
Index: src/backend/storage/large_object/inv_api.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/storage/large_object/inv_api.c,v
retrieving revision 1.132
diff -c -p -r1.132 inv_api.c
*** src/backend/storage/large_object/inv_api.c	12 Apr 2008 23:14:21 -0000	1.132
--- src/backend/storage/large_object/inv_api.c	14 Apr 2008 17:38:01 -0000
*************** inv_open(Oid lobjId, int flags, MemoryCo
*** 246,257 ****
  	}
  	else if (flags & INV_READ)
  	{
! 		/* be sure to copy snap into mcxt */
! 		MemoryContext oldContext = MemoryContextSwitchTo(mcxt);
! 
! 		retval->snapshot = CopySnapshot(ActiveSnapshot);
  		retval->flags = IFS_RDLOCK;
- 		MemoryContextSwitchTo(oldContext);
  	}
  	else
  		elog(ERROR, "invalid flags: %d", flags);
--- 246,253 ----
  	}
  	else if (flags & INV_READ)
  	{
! 		retval->snapshot = RegisterSnapshot(GetActiveSnapshot());
  		retval->flags = IFS_RDLOCK;
  	}
  	else
  		elog(ERROR, "invalid flags: %d", flags);
*************** inv_close(LargeObjectDesc *obj_desc)
*** 274,280 ****
  {
  	Assert(PointerIsValid(obj_desc));
  	if (obj_desc->snapshot != SnapshotNow)
! 		FreeSnapshot(obj_desc->snapshot);
  	pfree(obj_desc);
  }
  
--- 270,276 ----
  {
  	Assert(PointerIsValid(obj_desc));
  	if (obj_desc->snapshot != SnapshotNow)
! 		UnregisterSnapshot(obj_desc->snapshot);
  	pfree(obj_desc);
  }
  
Index: src/backend/tcop/fastpath.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/tcop/fastpath.c,v
retrieving revision 1.99
diff -c -p -r1.99 fastpath.c
*** src/backend/tcop/fastpath.c	26 Mar 2008 18:48:59 -0000	1.99
--- src/backend/tcop/fastpath.c	11 Apr 2008 18:30:48 -0000
*************** HandleFunctionRequest(StringInfo msgBuf)
*** 309,315 ****
  	 * Now that we know we are in a valid transaction, set snapshot in case
  	 * needed by function itself or one of the datatype I/O routines.
  	 */
! 	ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  
  	/*
  	 * Begin parsing the buffer contents.
--- 309,315 ----
  	 * Now that we know we are in a valid transaction, set snapshot in case
  	 * needed by function itself or one of the datatype I/O routines.
  	 */
! 	PushActiveSnapshot(GetTransactionSnapshot());
  
  	/*
  	 * Begin parsing the buffer contents.
*************** HandleFunctionRequest(StringInfo msgBuf)
*** 396,401 ****
--- 396,404 ----
  
  	SendFunctionResult(retval, fcinfo.isnull, fip->rettype, rformat);
  
+ 	/* We no longer need the snapshot */
+ 	PopActiveSnapshot();
+ 
  	/*
  	 * Emit duration logging if appropriate.
  	 */
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.548
diff -c -p -r1.548 postgres.c
*** src/backend/tcop/postgres.c	2 Apr 2008 18:31:50 -0000	1.548
--- src/backend/tcop/postgres.c	11 Apr 2008 17:08:54 -0000
*************** pg_plan_queries(List *querytrees, int cu
*** 732,744 ****
  				bool needSnapshot)
  {
  	List	   * volatile stmt_list = NIL;
! 	Snapshot	saveActiveSnapshot = ActiveSnapshot;
! 
! 	/* PG_TRY to ensure previous ActiveSnapshot is restored on error */
! 	PG_TRY();
! 	{
! 		Snapshot	mySnapshot = NULL;
! 		ListCell   *query_list;
  
  		foreach(query_list, querytrees)
  		{
--- 732,739 ----
  				bool needSnapshot)
  {
  	List	   * volatile stmt_list = NIL;
! 	ListCell   *query_list;
! 	bool		snapshot_set = false;
  
  		foreach(query_list, querytrees)
  		{
*************** pg_plan_queries(List *querytrees, int cu
*** 752,762 ****
  			}
  			else
  			{
! 				if (needSnapshot && mySnapshot == NULL)
  				{
! 					mySnapshot = CopySnapshot(GetTransactionSnapshot());
! 					ActiveSnapshot = mySnapshot;
  				}
  				stmt = (Node *) pg_plan_query(query, cursorOptions,
  											  boundParams);
  			}
--- 747,758 ----
  			}
  			else
  			{
! 				if (needSnapshot && !snapshot_set)
  				{
! 					PushActiveSnapshot(GetTransactionSnapshot());
! 					snapshot_set = true;
  				}
+ 
  				stmt = (Node *) pg_plan_query(query, cursorOptions,
  											  boundParams);
  			}
*************** pg_plan_queries(List *querytrees, int cu
*** 764,779 ****
  			stmt_list = lappend(stmt_list, stmt);
  		}
  
! 		if (mySnapshot)
! 			FreeSnapshot(mySnapshot);
! 	}
! 	PG_CATCH();
! 	{
! 		ActiveSnapshot = saveActiveSnapshot;
! 		PG_RE_THROW();
! 	}
! 	PG_END_TRY();
! 	ActiveSnapshot = saveActiveSnapshot;
  
  	return stmt_list;
  }
--- 760,767 ----
  			stmt_list = lappend(stmt_list, stmt);
  		}
  
! 	if (snapshot_set)
! 		PopActiveSnapshot();
  
  	return stmt_list;
  }
Index: src/backend/tcop/pquery.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/tcop/pquery.c,v
retrieving revision 1.122
diff -c -p -r1.122 pquery.c
*** src/backend/tcop/pquery.c	26 Mar 2008 18:48:59 -0000	1.122
--- src/backend/tcop/pquery.c	11 Apr 2008 18:30:19 -0000
*************** CreateQueryDesc(PlannedStmt *plannedstmt
*** 70,77 ****
  	qd->operation = plannedstmt->commandType;	/* operation */
  	qd->plannedstmt = plannedstmt;		/* plan */
  	qd->utilitystmt = plannedstmt->utilityStmt; /* in case DECLARE CURSOR */
! 	qd->snapshot = snapshot;	/* snapshot */
! 	qd->crosscheck_snapshot = crosscheck_snapshot;		/* RI check snapshot */
  	qd->dest = dest;			/* output dest */
  	qd->params = params;		/* parameter values passed into query */
  	qd->doInstrument = doInstrument;	/* instrumentation wanted? */
--- 70,78 ----
  	qd->operation = plannedstmt->commandType;	/* operation */
  	qd->plannedstmt = plannedstmt;		/* plan */
  	qd->utilitystmt = plannedstmt->utilityStmt; /* in case DECLARE CURSOR */
! 	qd->snapshot = RegisterSnapshot(snapshot);	/* snapshot */
! 	/* RI check snapshot */
! 	qd->crosscheck_snapshot = RegisterSnapshot(crosscheck_snapshot);
  	qd->dest = dest;			/* output dest */
  	qd->params = params;		/* parameter values passed into query */
  	qd->doInstrument = doInstrument;	/* instrumentation wanted? */
*************** CreateUtilityQueryDesc(Node *utilitystmt
*** 98,104 ****
  	qd->operation = CMD_UTILITY;	/* operation */
  	qd->plannedstmt = NULL;
  	qd->utilitystmt = utilitystmt;		/* utility command */
! 	qd->snapshot = snapshot;	/* snapshot */
  	qd->crosscheck_snapshot = InvalidSnapshot;	/* RI check snapshot */
  	qd->dest = dest;			/* output dest */
  	qd->params = params;		/* parameter values passed into query */
--- 99,105 ----
  	qd->operation = CMD_UTILITY;	/* operation */
  	qd->plannedstmt = NULL;
  	qd->utilitystmt = utilitystmt;		/* utility command */
! 	qd->snapshot = RegisterSnapshot(snapshot);	/* snapshot */
  	qd->crosscheck_snapshot = InvalidSnapshot;	/* RI check snapshot */
  	qd->dest = dest;			/* output dest */
  	qd->params = params;		/* parameter values passed into query */
*************** CreateUtilityQueryDesc(Node *utilitystmt
*** 118,123 ****
--- 119,127 ----
  void
  FreeQueryDesc(QueryDesc *qdesc)
  {
+ 	UnregisterSnapshot(qdesc->snapshot);
+ 	UnregisterSnapshot(qdesc->crosscheck_snapshot);
+ 
  	/* Can't be a live query */
  	Assert(qdesc->estate == NULL);
  	/* Only the QueryDesc itself need be freed */
*************** ProcessQuery(PlannedStmt *plan,
*** 152,167 ****
  	elog(DEBUG3, "ProcessQuery");
  
  	/*
! 	 * Must always set snapshot for plannable queries.	Note we assume that
! 	 * caller will take care of restoring ActiveSnapshot on exit/error.
  	 */
! 	ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  
  	/*
  	 * Create the QueryDesc object
  	 */
  	queryDesc = CreateQueryDesc(plan,
! 								ActiveSnapshot, InvalidSnapshot,
  								dest, params, false);
  
  	/*
--- 156,170 ----
  	elog(DEBUG3, "ProcessQuery");
  
  	/*
! 	 * Must always set a snapshot for plannable queries.
  	 */
! 	PushActiveSnapshot(GetTransactionSnapshot());
  
  	/*
  	 * Create the QueryDesc object
  	 */
  	queryDesc = CreateQueryDesc(plan,
! 								GetActiveSnapshot(), InvalidSnapshot,
  								dest, params, false);
  
  	/*
*************** ProcessQuery(PlannedStmt *plan,
*** 216,230 ****
  	/* Now take care of any queued AFTER triggers */
  	AfterTriggerEndQuery(queryDesc->estate);
  
  	/*
  	 * Now, we close down all the scans and free allocated resources.
  	 */
  	ExecutorEnd(queryDesc);
  
  	FreeQueryDesc(queryDesc);
- 
- 	FreeSnapshot(ActiveSnapshot);
- 	ActiveSnapshot = NULL;
  }
  
  /*
--- 219,232 ----
  	/* Now take care of any queued AFTER triggers */
  	AfterTriggerEndQuery(queryDesc->estate);
  
+ 	PopActiveSnapshot();
+ 
  	/*
  	 * Now, we close down all the scans and free allocated resources.
  	 */
  	ExecutorEnd(queryDesc);
  
  	FreeQueryDesc(queryDesc);
  }
  
  /*
*************** void
*** 446,452 ****
  PortalStart(Portal portal, ParamListInfo params, Snapshot snapshot)
  {
  	Portal		saveActivePortal;
- 	Snapshot	saveActiveSnapshot;
  	ResourceOwner saveResourceOwner;
  	MemoryContext savePortalContext;
  	MemoryContext oldContext;
--- 448,453 ----
*************** PortalStart(Portal portal, ParamListInfo
*** 460,472 ****
  	 * Set up global portal context pointers.
  	 */
  	saveActivePortal = ActivePortal;
- 	saveActiveSnapshot = ActiveSnapshot;
  	saveResourceOwner = CurrentResourceOwner;
  	savePortalContext = PortalContext;
  	PG_TRY();
  	{
  		ActivePortal = portal;
- 		ActiveSnapshot = NULL;	/* will be set later */
  		CurrentResourceOwner = portal->resowner;
  		PortalContext = PortalGetHeapMemory(portal);
  
--- 461,471 ----
*************** PortalStart(Portal portal, ParamListInfo
*** 492,507 ****
  				 * copy it into the portal's context.
  				 */
  				if (snapshot)
! 					ActiveSnapshot = CopySnapshot(snapshot);
  				else
! 					ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  
  				/*
  				 * Create QueryDesc in portal's context; for the moment, set
  				 * the destination to DestNone.
  				 */
  				queryDesc = CreateQueryDesc((PlannedStmt *) linitial(portal->stmts),
! 											ActiveSnapshot,
  											InvalidSnapshot,
  											None_Receiver,
  											params,
--- 491,506 ----
  				 * copy it into the portal's context.
  				 */
  				if (snapshot)
! 					PushActiveSnapshot(snapshot);
  				else
! 					PushActiveSnapshot(GetTransactionSnapshot());
  
  				/*
  				 * Create QueryDesc in portal's context; for the moment, set
  				 * the destination to DestNone.
  				 */
  				queryDesc = CreateQueryDesc((PlannedStmt *) linitial(portal->stmts),
! 											GetActiveSnapshot(),
  											InvalidSnapshot,
  											None_Receiver,
  											params,
*************** PortalStart(Portal portal, ParamListInfo
*** 545,550 ****
--- 544,551 ----
  				portal->atEnd = false;	/* allow fetches */
  				portal->portalPos = 0;
  				portal->posOverflow = false;
+ 
+ 				PopActiveSnapshot();
  				break;
  
  			case PORTAL_ONE_RETURNING:
*************** PortalStart(Portal portal, ParamListInfo
*** 608,614 ****
  
  		/* Restore global vars and propagate error */
  		ActivePortal = saveActivePortal;
- 		ActiveSnapshot = saveActiveSnapshot;
  		CurrentResourceOwner = saveResourceOwner;
  		PortalContext = savePortalContext;
  
--- 609,614 ----
*************** PortalStart(Portal portal, ParamListInfo
*** 619,625 ****
  	MemoryContextSwitchTo(oldContext);
  
  	ActivePortal = saveActivePortal;
- 	ActiveSnapshot = saveActiveSnapshot;
  	CurrentResourceOwner = saveResourceOwner;
  	PortalContext = savePortalContext;
  
--- 619,624 ----
*************** PortalRun(Portal portal, long count, boo
*** 707,713 ****
  	ResourceOwner saveTopTransactionResourceOwner;
  	MemoryContext saveTopTransactionContext;
  	Portal		saveActivePortal;
- 	Snapshot	saveActiveSnapshot;
  	ResourceOwner saveResourceOwner;
  	MemoryContext savePortalContext;
  	MemoryContext saveMemoryContext;
--- 706,711 ----
*************** PortalRun(Portal portal, long count, boo
*** 751,764 ****
  	saveTopTransactionResourceOwner = TopTransactionResourceOwner;
  	saveTopTransactionContext = TopTransactionContext;
  	saveActivePortal = ActivePortal;
- 	saveActiveSnapshot = ActiveSnapshot;
  	saveResourceOwner = CurrentResourceOwner;
  	savePortalContext = PortalContext;
  	saveMemoryContext = CurrentMemoryContext;
  	PG_TRY();
  	{
  		ActivePortal = portal;
- 		ActiveSnapshot = NULL;	/* will be set later */
  		CurrentResourceOwner = portal->resowner;
  		PortalContext = PortalGetHeapMemory(portal);
  
--- 749,760 ----
*************** PortalRun(Portal portal, long count, boo
*** 839,845 ****
  		else
  			MemoryContextSwitchTo(saveMemoryContext);
  		ActivePortal = saveActivePortal;
- 		ActiveSnapshot = saveActiveSnapshot;
  		if (saveResourceOwner == saveTopTransactionResourceOwner)
  			CurrentResourceOwner = TopTransactionResourceOwner;
  		else
--- 835,840 ----
*************** PortalRun(Portal portal, long count, boo
*** 855,861 ****
  	else
  		MemoryContextSwitchTo(saveMemoryContext);
  	ActivePortal = saveActivePortal;
- 	ActiveSnapshot = saveActiveSnapshot;
  	if (saveResourceOwner == saveTopTransactionResourceOwner)
  		CurrentResourceOwner = TopTransactionResourceOwner;
  	else
--- 850,855 ----
*************** PortalRunSelect(Portal portal,
*** 940,948 ****
  			nprocessed = RunFromStore(portal, direction, count, dest);
  		else
  		{
! 			ActiveSnapshot = queryDesc->snapshot;
  			ExecutorRun(queryDesc, direction, count);
  			nprocessed = queryDesc->estate->es_processed;
  		}
  
  		if (!ScanDirectionIsNoMovement(direction))
--- 934,943 ----
  			nprocessed = RunFromStore(portal, direction, count, dest);
  		else
  		{
! 			PushActiveSnapshot(queryDesc->snapshot);
  			ExecutorRun(queryDesc, direction, count);
  			nprocessed = queryDesc->estate->es_processed;
+ 			PopActiveSnapshot();
  		}
  
  		if (!ScanDirectionIsNoMovement(direction))
*************** PortalRunSelect(Portal portal,
*** 982,990 ****
  			nprocessed = RunFromStore(portal, direction, count, dest);
  		else
  		{
! 			ActiveSnapshot = queryDesc->snapshot;
  			ExecutorRun(queryDesc, direction, count);
  			nprocessed = queryDesc->estate->es_processed;
  		}
  
  		if (!ScanDirectionIsNoMovement(direction))
--- 977,986 ----
  			nprocessed = RunFromStore(portal, direction, count, dest);
  		else
  		{
! 			PushActiveSnapshot(queryDesc->snapshot);
  			ExecutorRun(queryDesc, direction, count);
  			nprocessed = queryDesc->estate->es_processed;
+ 			PopActiveSnapshot();
  		}
  
  		if (!ScanDirectionIsNoMovement(direction))
*************** static void
*** 1140,1145 ****
--- 1136,1143 ----
  PortalRunUtility(Portal portal, Node *utilityStmt, bool isTopLevel,
  				 DestReceiver *dest, char *completionTag)
  {
+ 	bool	active_snapshot_set;
+ 
  	elog(DEBUG3, "ProcessUtility");
  
  	/*
*************** PortalRunUtility(Portal portal, Node *ut
*** 1152,1160 ****
  	 * hacks.  Beware of listing anything that can modify the database --- if,
  	 * say, it has to update an index with expressions that invoke
  	 * user-defined functions, then it had better have a snapshot.
- 	 *
- 	 * Note we assume that caller will take care of restoring ActiveSnapshot
- 	 * on exit/error.
  	 */
  	if (!(IsA(utilityStmt, TransactionStmt) ||
  		  IsA(utilityStmt, LockStmt) ||
--- 1150,1155 ----
*************** PortalRunUtility(Portal portal, Node *ut
*** 1167,1175 ****
  		  IsA(utilityStmt, NotifyStmt) ||
  		  IsA(utilityStmt, UnlistenStmt) ||
  		  IsA(utilityStmt, CheckPointStmt)))
! 		ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  	else
! 		ActiveSnapshot = NULL;
  
  	ProcessUtility(utilityStmt,
  				   portal->sourceText,
--- 1162,1173 ----
  		  IsA(utilityStmt, NotifyStmt) ||
  		  IsA(utilityStmt, UnlistenStmt) ||
  		  IsA(utilityStmt, CheckPointStmt)))
! 	{
! 		PushActiveSnapshot(GetTransactionSnapshot());
! 		active_snapshot_set = true;
! 	}
  	else
! 		active_snapshot_set = false;
  
  	ProcessUtility(utilityStmt,
  				   portal->sourceText,
*************** PortalRunUtility(Portal portal, Node *ut
*** 1181,1189 ****
  	/* Some utility statements may change context on us */
  	MemoryContextSwitchTo(PortalGetHeapMemory(portal));
  
! 	if (ActiveSnapshot)
! 		FreeSnapshot(ActiveSnapshot);
! 	ActiveSnapshot = NULL;
  }
  
  /*
--- 1179,1193 ----
  	/* Some utility statements may change context on us */
  	MemoryContextSwitchTo(PortalGetHeapMemory(portal));
  
! 	/*
! 	 * Some utility commands may pop the ActiveSnapshot stack from under us,
! 	 * so we only pop the stack if we actually see a snapshot set.  Note that
! 	 * the set of utility commands that do this must be the same set
! 	 * disallowed to run inside a transaction; otherwise, we could be popping
! 	 * a snapshot that belong to some other operation.
! 	 */
! 	if (active_snapshot_set && ActiveSnapshotSet())
! 		PopActiveSnapshot();
  }
  
  /*
*************** PortalRunFetch(Portal portal,
*** 1321,1327 ****
  {
  	long		result;
  	Portal		saveActivePortal;
- 	Snapshot	saveActiveSnapshot;
  	ResourceOwner saveResourceOwner;
  	MemoryContext savePortalContext;
  	MemoryContext oldContext;
--- 1325,1330 ----
*************** PortalRunFetch(Portal portal,
*** 1341,1353 ****
  	 * Set up global portal context pointers.
  	 */
  	saveActivePortal = ActivePortal;
- 	saveActiveSnapshot = ActiveSnapshot;
  	saveResourceOwner = CurrentResourceOwner;
  	savePortalContext = PortalContext;
  	PG_TRY();
  	{
  		ActivePortal = portal;
- 		ActiveSnapshot = NULL;	/* will be set later */
  		CurrentResourceOwner = portal->resowner;
  		PortalContext = PortalGetHeapMemory(portal);
  
--- 1344,1354 ----
*************** PortalRunFetch(Portal portal,
*** 1388,1394 ****
  
  		/* Restore global vars and propagate error */
  		ActivePortal = saveActivePortal;
- 		ActiveSnapshot = saveActiveSnapshot;
  		CurrentResourceOwner = saveResourceOwner;
  		PortalContext = savePortalContext;
  
--- 1389,1394 ----
*************** PortalRunFetch(Portal portal,
*** 1402,1408 ****
  	portal->status = PORTAL_READY;
  
  	ActivePortal = saveActivePortal;
- 	ActiveSnapshot = saveActiveSnapshot;
  	CurrentResourceOwner = saveResourceOwner;
  	PortalContext = savePortalContext;
  
--- 1402,1407 ----
Index: src/backend/utils/adt/ri_triggers.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/adt/ri_triggers.c,v
retrieving revision 1.107
diff -c -p -r1.107 ri_triggers.c
*** src/backend/utils/adt/ri_triggers.c	26 Mar 2008 21:10:39 -0000	1.107
--- src/backend/utils/adt/ri_triggers.c	11 Apr 2008 17:08:54 -0000
*************** RI_Initial_Check(Trigger *trigger, Relat
*** 2756,2767 ****
  	/*
  	 * Run the plan.  For safety we force a current snapshot to be used. (In
  	 * serializable mode, this arguably violates serializability, but we
! 	 * really haven't got much choice.)  We need at most one tuple returned,
! 	 * so pass limit = 1.
  	 */
  	spi_result = SPI_execute_snapshot(qplan,
  									  NULL, NULL,
! 									  CopySnapshot(GetLatestSnapshot()),
  									  InvalidSnapshot,
  									  true, false, 1);
  
--- 2756,2768 ----
  	/*
  	 * Run the plan.  For safety we force a current snapshot to be used. (In
  	 * serializable mode, this arguably violates serializability, but we
! 	 * really haven't got much choice.)  We don't need to register the
! 	 * snapshot, because SPI_execute_snapshot will see to it.  We need at most
! 	 * one tuple returned, so pass limit = 1.
  	 */
  	spi_result = SPI_execute_snapshot(qplan,
  									  NULL, NULL,
! 									  GetLatestSnapshot(),
  									  InvalidSnapshot,
  									  true, false, 1);
  
*************** ri_PerformCheck(RI_QueryKey *qkey, SPIPl
*** 3311,3323 ****
  	 * caller passes detectNewRows == false then it's okay to do the query
  	 * with the transaction snapshot; otherwise we use a current snapshot, and
  	 * tell the executor to error out if it finds any rows under the current
! 	 * snapshot that wouldn't be visible per the transaction snapshot.
  	 */
  	if (IsXactIsoLevelSerializable && detectNewRows)
  	{
  		CommandCounterIncrement();		/* be sure all my own work is visible */
! 		test_snapshot = CopySnapshot(GetLatestSnapshot());
! 		crosscheck_snapshot = CopySnapshot(GetTransactionSnapshot());
  	}
  	else
  	{
--- 3312,3326 ----
  	 * caller passes detectNewRows == false then it's okay to do the query
  	 * with the transaction snapshot; otherwise we use a current snapshot, and
  	 * tell the executor to error out if it finds any rows under the current
! 	 * snapshot that wouldn't be visible per the transaction snapshot.  Note
! 	 * that SPI_execute_snapshot will register the snapshots, so we don't need
! 	 * to bother here.
  	 */
  	if (IsXactIsoLevelSerializable && detectNewRows)
  	{
  		CommandCounterIncrement();		/* be sure all my own work is visible */
! 		test_snapshot = GetLatestSnapshot();
! 		crosscheck_snapshot = GetTransactionSnapshot();
  	}
  	else
  	{
Index: src/backend/utils/adt/txid.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/adt/txid.c,v
retrieving revision 1.6
diff -c -p -r1.6 txid.c
*** src/backend/utils/adt/txid.c	26 Mar 2008 18:48:59 -0000	1.6
--- src/backend/utils/adt/txid.c	11 Apr 2008 17:07:51 -0000
*************** txid_current_snapshot(PG_FUNCTION_ARGS)
*** 362,368 ****
  	TxidEpoch	state;
  	Snapshot	cur;
  
! 	cur = ActiveSnapshot;
  	if (cur == NULL)
  		elog(ERROR, "txid_current_snapshot: ActiveSnapshot == NULL");
  
--- 362,368 ----
  	TxidEpoch	state;
  	Snapshot	cur;
  
! 	cur = GetActiveSnapshot();
  	if (cur == NULL)
  		elog(ERROR, "txid_current_snapshot: ActiveSnapshot == NULL");
  
Index: src/backend/utils/cache/plancache.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/cache/plancache.c,v
retrieving revision 1.17
diff -c -p -r1.17 plancache.c
*** src/backend/utils/cache/plancache.c	26 Mar 2008 18:48:59 -0000	1.17
--- src/backend/utils/cache/plancache.c	11 Apr 2008 17:07:51 -0000
*************** do_planning(List *querytrees, int cursor
*** 550,580 ****
  	/*
  	 * If a snapshot is already set (the normal case), we can just use that
  	 * for planning.  But if it isn't, we have to tell pg_plan_queries to make
! 	 * a snap if it needs one.	In that case we should arrange to reset
! 	 * ActiveSnapshot afterward, to ensure that RevalidateCachedPlan has no
! 	 * caller-visible effects on the snapshot.	Having to replan is an unusual
! 	 * case, and it seems a really bad idea for RevalidateCachedPlan to affect
! 	 * the snapshot only in unusual cases.	(Besides, the snap might have been
! 	 * created in a short-lived context.)
  	 */
! 	if (ActiveSnapshot != NULL)
! 		stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL, false);
! 	else
! 	{
! 		PG_TRY();
! 		{
! 			stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL, true);
! 		}
! 		PG_CATCH();
! 		{
! 			/* Restore global vars and propagate error */
! 			ActiveSnapshot = NULL;
! 			PG_RE_THROW();
! 		}
! 		PG_END_TRY();
! 
! 		ActiveSnapshot = NULL;
! 	}
  
  	return stmt_list;
  }
--- 550,560 ----
  	/*
  	 * If a snapshot is already set (the normal case), we can just use that
  	 * for planning.  But if it isn't, we have to tell pg_plan_queries to make
! 	 * a snap if it needs one.	pg_plan_queries is also responsible for making
! 	 * sure ActiveSnapshot is reset in that case.
  	 */
! 	stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL,
! 								!ActiveSnapshotSet());
  
  	return stmt_list;
  }
Index: src/backend/utils/time/snapmgr.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/time/snapmgr.c,v
retrieving revision 1.1
diff -c -p -r1.1 snapmgr.c
*** src/backend/utils/time/snapmgr.c	26 Mar 2008 18:48:59 -0000	1.1
--- src/backend/utils/time/snapmgr.c	14 Apr 2008 23:44:45 -0000
***************
*** 14,22 ****
--- 14,25 ----
  
  #include "access/xact.h"
  #include "access/transam.h"
+ #include "storage/proc.h"
  #include "storage/procarray.h"
+ #include "utils/memutils.h"
  #include "utils/snapmgr.h"
  #include "utils/tqual.h"
+ #include "utils/memutils.h"
  
  
  /*
*************** Snapshot	SerializableSnapshot = NULL;
*** 31,43 ****
  Snapshot	LatestSnapshot = NULL;
  
  /*
-  * This pointer is not maintained by this module, but it's convenient
-  * to declare it here anyway.  Callers typically assign a copy of
-  * GetTransactionSnapshot's result to ActiveSnapshot.
-  */
- Snapshot	ActiveSnapshot = NULL;
- 
- /*
   * These are updated by GetSnapshotData.  We initialize them this way
   * for the convenience of TransactionIdIsInProgress: even in bootstrap
   * mode, we don't want it to say that BootstrapTransactionId is in progress.
--- 34,39 ----
*************** TransactionId TransactionXmin = FirstNor
*** 46,51 ****
--- 42,71 ----
  TransactionId RecentXmin = FirstNormalTransactionId;
  TransactionId RecentGlobalXmin = FirstNormalTransactionId;
  
+ /* Elements of the active snapshot stack */
+ typedef struct ActiveSnapshotElt
+ {
+ 	Snapshot	as_snap;
+ 	char	   *as_name;
+ 	struct ActiveSnapshotElt *as_next;
+ } ActiveSnapshotElt;
+ 
+ /* Elements of the list of registered snapshots */
+ typedef struct RegdSnapshotElt
+ {
+ 	Snapshot	s_snap;
+ 	struct RegdSnapshotElt	*s_next;
+ } RegdSnapshotElt;
+ 
+ /* Head of the list of registered snapshots */
+ static RegdSnapshotElt	   *RegisteredSnapshotList = NULL;
+ 
+ /* Top of the stack of active snapshots */
+ static ActiveSnapshotElt	*ActiveSnapshot = NULL;
+ 
+ /* Memory context where tracked snapshots reside */
+ static MemoryContext		SnapshotContext = NULL;
+ 
  
  /*
   * GetTransactionSnapshot
*************** GetLatestSnapshot(void)
*** 95,104 ****
  }
  
  /*
   * CopySnapshot
   *		Copy the given snapshot.
   *
!  * The copy is palloc'd in the current memory context.
   */
  Snapshot
  CopySnapshot(Snapshot snapshot)
--- 115,141 ----
  }
  
  /*
+  * Initialize the private memory context for snapshots
+  */
+ static void
+ SnapshotMemoryInit(void)
+ {
+ 	if (SnapshotContext != NULL)
+ 		return;
+ 
+ 	SnapshotContext = AllocSetContextCreate(TopTransactionContext,
+ 											"Snapshot Context",
+ 											ALLOCSET_DEFAULT_MINSIZE,
+ 											ALLOCSET_DEFAULT_INITSIZE,
+ 											ALLOCSET_DEFAULT_MAXSIZE);
+ }
+ 
+ /*
   * CopySnapshot
   *		Copy the given snapshot.
   *
!  * The copy is palloc'd in SnapshotContext and has initial refcounts and
!  * subtransaction level set to 0.  These snapshots have the copied flag set.
   */
  Snapshot
  CopySnapshot(Snapshot snapshot)
*************** CopySnapshot(Snapshot snapshot)
*** 107,121 ****
  	Size		subxipoff;
  	Size		size;
  
  	/* We allocate any XID arrays needed in the same palloc block. */
  	size = subxipoff = sizeof(SnapshotData) +
  		snapshot->xcnt * sizeof(TransactionId);
  	if (snapshot->subxcnt > 0)
  		size += snapshot->subxcnt * sizeof(TransactionId);
  
! 	newsnap = (Snapshot) palloc(size);
  	memcpy(newsnap, snapshot, sizeof(SnapshotData));
  
  	/* setup XID array */
  	if (snapshot->xcnt > 0)
  	{
--- 144,168 ----
  	Size		subxipoff;
  	Size		size;
  
+ 	Assert(snapshot != InvalidSnapshot);
+ 
+ 	if (SnapshotContext == NULL)
+ 		SnapshotMemoryInit();
+ 
  	/* We allocate any XID arrays needed in the same palloc block. */
  	size = subxipoff = sizeof(SnapshotData) +
  		snapshot->xcnt * sizeof(TransactionId);
  	if (snapshot->subxcnt > 0)
  		size += snapshot->subxcnt * sizeof(TransactionId);
  
! 	newsnap = (Snapshot) MemoryContextAlloc(SnapshotContext, size);
  	memcpy(newsnap, snapshot, sizeof(SnapshotData));
  
+ 	newsnap->regd_count = 0;
+ 	newsnap->active_count = 0;
+ 	newsnap->level = 0;
+ 	newsnap->copied = true;
+ 
  	/* setup XID array */
  	if (snapshot->xcnt > 0)
  	{
*************** CopySnapshot(Snapshot snapshot)
*** 141,156 ****
  
  /*
   * FreeSnapshot
!  *		Free a snapshot previously copied with CopySnapshot.
   *
!  * This is currently identical to pfree, but is provided for cleanliness.
!  *
!  * Do *not* apply this to the results of GetTransactionSnapshot or
!  * GetLatestSnapshot, since those are just static structs.
   */
  void
  FreeSnapshot(Snapshot snapshot)
  {
  	pfree(snapshot);
  }
  
--- 188,204 ----
  
  /*
   * FreeSnapshot
!  *		Free the memory associated with a snapshot.
   *
!  * There shouldn't be any need to call this outside this module, as it is
!  * called as soon as the snapshot's refcount goes down to 0.
   */
  void
  FreeSnapshot(Snapshot snapshot)
  {
+ 	Assert(snapshot->regd_count == 0);
+ 	Assert(snapshot->active_count == 0);
+ 
  	pfree(snapshot);
  }
  
*************** FreeXactSnapshot(void)
*** 168,172 ****
  	 */
  	SerializableSnapshot = NULL;
  	LatestSnapshot = NULL;
! 	ActiveSnapshot = NULL;		/* just for cleanliness */
  }
--- 216,533 ----
  	 */
  	SerializableSnapshot = NULL;
  	LatestSnapshot = NULL;
! }
! 
! /*
!  * PushActiveSnapshot
!  * 		Set the given snapshot as the current active snapshot
!  *
!  * If this is the first use of this snapshot, create a new long-lived copy with
!  * active refcount=1.  Otherwise, only increment the refcount.
!  */
! void
! PushActiveSnapshot(Snapshot snap)
! {
! 	ActiveSnapshotElt	*newactive;
! 
! 	Assert(snap != InvalidSnapshot);
! 
! 	if (SnapshotContext == NULL)
! 		SnapshotMemoryInit();
! 
! 	/* Static snapshot?  Create a persistent copy */
! 	snap = snap->copied ? snap : CopySnapshot(snap);
! 
! 	newactive = MemoryContextAlloc(SnapshotContext, sizeof(ActiveSnapshotElt));
! 	newactive->as_snap = snap;
! 	newactive->as_next = ActiveSnapshot;
! 
! 	if (newactive->as_snap->level == 0)
! 		newactive->as_snap->level = GetCurrentTransactionNestLevel();
! 	newactive->as_snap->active_count++;
! 
! 	ActiveSnapshot = newactive;
! }
! 
! /*
!  * PopActiveSnapshot
!  *
!  * Remove the topmost snapshot from the active snapshot stack, decrementing the
!  * reference count, and free it if this was the last reference.
!  */
! void
! PopActiveSnapshot(void)
! {
! 	ActiveSnapshotElt	*newstack;
! 
! 	newstack = ActiveSnapshot->as_next;
! 
! 	ActiveSnapshot->as_snap->active_count--;
! 
! 	if (ActiveSnapshot->as_snap->active_count == 0 &&
! 		ActiveSnapshot->as_snap->regd_count == 0)
! 		FreeSnapshot(ActiveSnapshot->as_snap);
! 
! 	ActiveSnapshot = newstack;
! }
! 
! Snapshot
! GetActiveSnapshot(void)
! {
! 	Assert(ActiveSnapshot != NULL);
! 
! 	return ActiveSnapshot->as_snap;
! }
! 
! bool
! ActiveSnapshotSet(void)
! {
! 	return ActiveSnapshot != NULL;
! }
! 
! /*
!  * RegisterSnapshot
!  * 		Register a snapshot as being in use
!  *
!  * This registers a snapshot on our list.  If the snapshot has been registered
!  * previously, increment the reference count.
!  *
!  * If InvalidSnapshot or a non-MVCC snapshot is passed, it is not registered.
!  * XXX -- should it be a hard error if a non-MVCC snapshot is passed?
!  */
! Snapshot
! RegisterSnapshot(Snapshot snapshot)
! {
! 	RegdSnapshotElt	*elt;
! 	RegdSnapshotElt	*newhead;
! 
! 	if (snapshot == InvalidSnapshot)
! 		return InvalidSnapshot;
! 
! 	SnapshotMemoryInit();
! 
! 	for (elt = RegisteredSnapshotList; elt != NULL; elt = elt->s_next)
! 	{
! 		if (elt->s_snap == snapshot)
! 		{
! 			elt->s_snap->regd_count++;
! 			return elt->s_snap;
! 		}
! 	}
! 
! 	/*
! 	 * Create the new list element.  If the active count of the snapshot is
! 	 * zero, then we must copy the snapshot to persistent memory (we already
! 	 * checked that it is not on the registered list either, so it must be
! 	 * new).  Otherwise we can just increment the reference count.
! 	 */
! 	newhead = MemoryContextAlloc(SnapshotContext, sizeof(RegdSnapshotElt));
! 	newhead->s_next = RegisteredSnapshotList;
! 	/* Static snapshot?  Create a persistent copy */
! 	newhead->s_snap = snapshot->copied ? snapshot : CopySnapshot(snapshot);
! 
! 	if (newhead->s_snap->level == 0)
! 		newhead->s_snap->level = GetCurrentTransactionNestLevel();
! 	newhead->s_snap->regd_count++;
! 
! 	RegisteredSnapshotList = newhead;
! 
! 	return RegisteredSnapshotList->s_snap;
! }
! 
! /*
!  * UnregisterSnapshot
!  * 		Signals that a snapshot is no longer necessary
!  *
!  * If both reference counts falls to zero, the snapshot memory is released.
!  * If only the registered list refcount falls to zero, just the list element is
!  * freed.
!  */
! void
! UnregisterSnapshot(Snapshot snapshot)
! {
! 	RegdSnapshotElt	*prev = NULL;
! 	RegdSnapshotElt	*elt;
! 	bool		found = false;
! 
! 	if (snapshot == InvalidSnapshot)
! 		return;
! 
! 	for (elt = RegisteredSnapshotList; elt != NULL; elt = elt->s_next)
! 	{
! 		if (elt->s_snap == snapshot)
! 		{
! 			elt->s_snap->regd_count--;
! 			found = true;
! 
! 			if (elt->s_snap->regd_count == 0)
! 			{
! 				/* delink it from the registered snapshot list */
! 				if (prev)
! 					prev->s_next = elt->s_next;
! 				else
! 					RegisteredSnapshotList = elt->s_next;
! 
! 				/* free the snapshot itself if it's no longer relevant */
! 				if (elt->s_snap->active_count == 0)
! 					FreeSnapshot(elt->s_snap);
! 
! 				/* and free the list element */
! 				pfree(elt);
! 			}
! 
! 			break;
! 		}
! 
! 		prev = elt;
! 	}
! 
! 	if (!found)
! 		elog(WARNING, "unregistering failed for snapshot %p", snapshot);
! }
! 
! /*
!  * AtSubCommit_Snapshot
!  */
! void
! AtSubCommit_Snapshot(void)
! {
! 	ActiveSnapshotElt	*active;
! 	RegdSnapshotElt	*regd;
! 	int		level;
! 
! 	if (SnapshotContext == NULL)
! 		return;
! 
! 	level = GetCurrentTransactionNestLevel();
! 
! 	/*
! 	 * Relabel the active snapshots set in this subtransaction as though they
! 	 * are owned by the parent subxact.
! 	 */
! 	for (active = ActiveSnapshot; active != NULL; active = active->as_next)
! 	{
! 		if (active->as_snap->level < level)
! 			break;
! 		active->as_snap->level = level - 1;
! 	}
! 
! 	/* Reassign all registered snapshots to the parent subxact. */
! 	for (regd = RegisteredSnapshotList; regd != NULL; regd = regd->s_next)
! 	{
! 		if (regd->s_snap->level == level)
! 			regd->s_snap->level--;
! 	}
! }
! 
! /*
!  * AtSubAbort_Snapshot
!  * 		Clean up snapshots after a subtransaction abort
!  */
! void
! AtSubAbort_Snapshot(void)
! {
! 	int		level;
! 	RegdSnapshotElt	*prev;
! 	RegdSnapshotElt	*regd;
! 
! 	level = GetCurrentTransactionNestLevel();
! 
! 	/* Forget the active snapshots set by this subtransaction */
! 	while (ActiveSnapshot && ActiveSnapshot->as_snap->level >= level)
! 	{
! 		ActiveSnapshotElt	*next;
! 
! 		next = ActiveSnapshot->as_next;
! 
! 		/*
! 		 * If it's still registered, we can't free it yet; mark it as not
! 		 * active so that it's freed below.  Otherwise do away with it.
! 		 */
! 		ActiveSnapshot->as_snap->active_count = 0;
! 		if (ActiveSnapshot->as_snap->regd_count == 0)
! 			FreeSnapshot(ActiveSnapshot->as_snap);
! 
! 		/* and free the stack element */
! 		pfree(ActiveSnapshot);
! 
! 		ActiveSnapshot = next;
! 	}
! 
! 	/* Unregister all snapshots registered during this subtransaction */
! 	prev = NULL;
! 	for (regd = RegisteredSnapshotList; regd != NULL; )
! 	{
! 		if (regd->s_snap->level == level)
! 		{
! 			RegdSnapshotElt	*tofree;
! 
! 			if (prev)
! 				prev->s_next = regd->s_next;
! 			else
! 				RegisteredSnapshotList = regd->s_next;
! 
! 			tofree = regd;
! 			regd = regd->s_next;
! 
! 			/* must be OK to free at this point */
! 			Assert(tofree->s_snap->active_count == 0);
! 			tofree->s_snap->regd_count = 0;
! 			FreeSnapshot(tofree->s_snap);
! 
! 			/* and free the list element */
! 			pfree(tofree);
! 		}
! 		else
! 		{
! 			prev = regd;
! 			regd = regd->s_next;
! 		}
! 	}
! }
! 
! /*
!  * AtEOXact_Snapshot
!  * 		Snapshot manager's cleanup function for end of transaction
!  *
!  * We just need to reset our memory context -- the memory itself will be freed
!  * with TopTransactionContext.
!  */
! void
! AtEOXact_Snapshot(bool isCommit)
! {
! 	if (SnapshotContext == NULL)
! 		return;
! 
! 	/* On commit, complain about leftover snapshots */
! 	if (isCommit)
! 	{
! 		ActiveSnapshotElt	*active;
! 		RegdSnapshotElt	*regd;
! 
! 		/* complain about unpopped active snapshots */
! 		for (active = ActiveSnapshot; active != NULL; active = active->as_next)
! 		{
! 			ereport(WARNING,
! 					(errmsg("snapshot %p still active", active)));
! 		}
! 	
! 
! 		/* complain about any unregistered snapshot */
! 		for (regd = RegisteredSnapshotList; regd != NULL; regd = regd->s_next)
! 		{
! 			ereport(WARNING,
! 					(errmsg("snapshot %p not destroyed at commit (%d regd refs, %d active refs)",
! 							regd->s_snap, regd->s_snap->regd_count,
! 							regd->s_snap->active_count)));
! 		}
! 	}
! 
! 	/*
! 	 * And forget about them.  We don't need to reset the context -- it'll
! 	 * go away with TopTransactionContext.
! 	 */
! 	SnapshotContext = NULL;
! 	ActiveSnapshot = NULL;
! 	RegisteredSnapshotList = NULL;
  }
Index: src/include/utils/snapmgr.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/utils/snapmgr.h,v
retrieving revision 1.1
diff -c -p -r1.1 snapmgr.h
*** src/include/utils/snapmgr.h	26 Mar 2008 18:48:59 -0000	1.1
--- src/include/utils/snapmgr.h	11 Apr 2008 19:49:26 -0000
***************
*** 18,24 ****
  
  extern PGDLLIMPORT Snapshot SerializableSnapshot;
  extern PGDLLIMPORT Snapshot LatestSnapshot;
- extern PGDLLIMPORT Snapshot ActiveSnapshot;
  
  extern TransactionId TransactionXmin;
  extern TransactionId RecentXmin;
--- 18,23 ----
*************** extern Snapshot CopySnapshot(Snapshot sn
*** 30,33 ****
--- 29,44 ----
  extern void FreeSnapshot(Snapshot snapshot);
  extern void FreeXactSnapshot(void);
  
+ extern void PushActiveSnapshot(Snapshot snapshot);
+ extern void PopActiveSnapshot(void);
+ extern Snapshot GetActiveSnapshot(void);
+ extern bool ActiveSnapshotSet(void);
+ 
+ extern Snapshot RegisterSnapshot(Snapshot snapshot);
+ extern void UnregisterSnapshot(Snapshot snapshot);
+ 
+ extern void AtSubCommit_Snapshot(void);
+ extern void AtSubAbort_Snapshot(void);
+ extern void AtEOXact_Snapshot(bool isCommit);
+ 
  #endif /* SNAPMGR_H */
Index: src/include/utils/snapshot.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/utils/snapshot.h,v
retrieving revision 1.2
diff -c -p -r1.2 snapshot.h
*** src/include/utils/snapshot.h	26 Mar 2008 21:10:39 -0000	1.2
--- src/include/utils/snapshot.h	14 Apr 2008 23:45:34 -0000
*************** typedef struct SnapshotData
*** 57,62 ****
--- 57,66 ----
  	 * out any that are >= xmax
  	 */
  	CommandId	curcid;			/* in my xact, CID < curcid are visible */
+ 	uint16		active_count;	/* refcount on ActiveSnapshot stack */
+ 	uint16		regd_count;		/* refcount on RegisteredSnapshotList */
+ 	int			level;			/* creating subtransaction level */
+ 	bool		copied;			/* false if it's a static snapshot */
  } SnapshotData;
  
  /*
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.209
diff -c -p -r1.209 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c	6 Apr 2008 23:43:29 -0000	1.209
--- src/pl/plpgsql/src/pl_exec.c	11 Apr 2008 17:08:54 -0000
*************** exec_eval_simple_expr(PLpgSQL_execstate 
*** 4164,4170 ****
  	CachedPlan *cplan;
  	ParamListInfo paramLI;
  	int			i;
! 	Snapshot	saveActiveSnapshot;
  
  	/*
  	 * Forget it if expression wasn't simple before.
--- 4164,4170 ----
  	CachedPlan *cplan;
  	ParamListInfo paramLI;
  	int			i;
! 	MemoryContext oldcontext;
  
  	/*
  	 * Forget it if expression wasn't simple before.
*************** exec_eval_simple_expr(PLpgSQL_execstate 
*** 4253,4269 ****
  	 * updates made so far by our own function.
  	 */
  	SPI_push();
- 	saveActiveSnapshot = ActiveSnapshot;
- 
- 	PG_TRY();
- 	{
- 		MemoryContext oldcontext;
  
  		oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
  		if (!estate->readonly_func)
  		{
  			CommandCounterIncrement();
! 			ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
  		}
  
  		/*
--- 4253,4264 ----
  	 * updates made so far by our own function.
  	 */
  	SPI_push();
  
  		oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
  		if (!estate->readonly_func)
  		{
  			CommandCounterIncrement();
! 			PushActiveSnapshot(GetTransactionSnapshot());
  		}
  
  		/*
*************** exec_eval_simple_expr(PLpgSQL_execstate 
*** 4274,4289 ****
  							   isNull,
  							   NULL);
  		MemoryContextSwitchTo(oldcontext);
- 	}
- 	PG_CATCH();
- 	{
- 		/* Restore global vars and propagate error */
- 		ActiveSnapshot = saveActiveSnapshot;
- 		PG_RE_THROW();
- 	}
- 	PG_END_TRY();
  
! 	ActiveSnapshot = saveActiveSnapshot;
  	SPI_pop();
  
  	/*
--- 4269,4278 ----
  							   isNull,
  							   NULL);
  		MemoryContextSwitchTo(oldcontext);
  
! 	if (!estate->readonly_func)
! 		PopActiveSnapshot();
! 
  	SPI_pop();
  
  	/*