From 39db90e0f4a420f2a76a9407373555697c450af5 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 6 Feb 2007 22:49:42 +0000
Subject: [PATCH] Fix an error in the original coding of holdable cursors:
 PersistHoldablePortal thought that it didn't have to reposition the
 underlying tuplestore if the portal is atEnd.  But this is not so, because
 tuplestores have separate read and write cursors ... and the read cursor
 hasn't moved from the start. This mistake explains bug #2970 from William
 Zhang.

Note: the coding here is pretty inefficient, but given that no one has noticed
this bug until now, I'd say hardly anyone uses the case where the cursor has
been advanced before being persisted.  So maybe it's not worth worrying about.
---
 src/backend/commands/portalcmds.c | 32 +++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 7b040b438b..28e270d365 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -309,6 +309,7 @@ PersistHoldablePortal(Portal portal)
 {
 	QueryDesc  *queryDesc = PortalGetQueryDesc(portal);
 	Portal		saveActivePortal;
+	Snapshot	saveActiveSnapshot;
 	ResourceOwner saveResourceOwner;
 	MemoryContext savePortalContext;
 	MemoryContext saveQueryContext;
@@ -350,12 +351,14 @@ PersistHoldablePortal(Portal portal)
 	 * Set up global portal context pointers.
 	 */
 	saveActivePortal = ActivePortal;
+	saveActiveSnapshot = ActiveSnapshot;
 	saveResourceOwner = CurrentResourceOwner;
 	savePortalContext = PortalContext;
 	saveQueryContext = QueryContext;
 	PG_TRY();
 	{
 		ActivePortal = portal;
+		ActiveSnapshot = queryDesc->snapshot;
 		CurrentResourceOwner = portal->resowner;
 		PortalContext = PortalGetHeapMemory(portal);
 		QueryContext = portal->queryContext;
@@ -386,15 +389,30 @@ PersistHoldablePortal(Portal portal)
 		/* we do not need AfterTriggerEndQuery() here */
 
 		/*
-		 * Reset the position in the result set: ideally, this could be
-		 * implemented by just skipping straight to the tuple # that we
-		 * need to be at, but the tuplestore API doesn't support that. So
-		 * we start at the beginning of the tuplestore and iterate through
-		 * it until we reach where we need to be.  FIXME someday?
+		 * Set the position in the result set: ideally, this could be
+		 * implemented by just skipping straight to the tuple # that we need
+		 * to be at, but the tuplestore API doesn't support that. So we start
+		 * at the beginning of the tuplestore and iterate through it until we
+		 * reach where we need to be.  FIXME someday?  (Fortunately, the
+		 * typical case is that we're supposed to be at or near the start
+		 * of the result set, so this isn't as bad as it sounds.)
 		 */
 		MemoryContextSwitchTo(portal->holdContext);
 
-		if (!portal->atEnd)
+		if (portal->atEnd)
+		{
+			/* we can handle this case even if posOverflow */
+			HeapTuple	tup;
+			bool		should_free;
+
+			while ((tup = tuplestore_gettuple(portal->holdStore, true,
+											  &should_free)) != NULL)
+			{
+				if (should_free)
+					pfree(tup);
+			}
+		}
+		else
 		{
 			long		store_pos;
 
@@ -428,6 +446,7 @@ PersistHoldablePortal(Portal portal)
 
 		/* Restore global vars and propagate error */
 		ActivePortal = saveActivePortal;
+		ActiveSnapshot = saveActiveSnapshot;
 		CurrentResourceOwner = saveResourceOwner;
 		PortalContext = savePortalContext;
 		QueryContext = saveQueryContext;
@@ -442,6 +461,7 @@ PersistHoldablePortal(Portal portal)
 	portal->status = PORTAL_READY;
 
 	ActivePortal = saveActivePortal;
+	ActiveSnapshot = saveActiveSnapshot;
 	CurrentResourceOwner = saveResourceOwner;
 	PortalContext = savePortalContext;
 	QueryContext = saveQueryContext;
-- 
2.39.5