Fix interaction between materializing holdable cursors and firing

deferred triggers: either one can create more work for the other,
so we have to loop till it's all gone.  Per example from andrew@supernews.
Add a regression test to help spot trouble in this area in future.

1 files changed, 59 insertions, 33 deletions

diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 8142a3bccab..f6156e63c37 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.76.4.1 2005/01/26 23:20:37 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.76.4.2 2005/04/11 19:51:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -416,18 +416,14 @@ DropDependentPortals(MemoryContext queryContext)
  *
  * Any holdable cursors created in this transaction need to be converted to
  * materialized form, since we are going to close down the executor and
- * release locks.  Remove all other portals created in this transaction.
- * Portals remaining from prior transactions should be left untouched.
+ * release locks.  Other portals are not touched yet.
  *
- * XXX This assumes that portals can be deleted in a random order, ie,
- * no portal has a reference to any other (at least not one that will be
- * exercised during deletion).	I think this is okay at the moment, but
- * we've had bugs of that ilk in the past.  Keep a close eye on cursor
- * references...
+ * Returns TRUE if any holdable cursors were processed, FALSE if not.
  */
-void
-AtCommit_Portals(void)
+bool
+CommitHoldablePortals(void)
 {
+	bool result = false;
 	HASH_SEQ_STATUS status;
 	PortalHashEnt *hentry;
 
@@ -437,27 +433,9 @@ AtCommit_Portals(void)
 	{
 		Portal		portal = hentry->portal;
 
-		/*
-		 * Do not touch active portals --- this can only happen in the
-		 * case of a multi-transaction utility command, such as VACUUM.
-		 *
-		 * Note however that any resource owner attached to such a portal is
-		 * still going to go away, so don't leave a dangling pointer.
-		 */
-		if (portal->status == PORTAL_ACTIVE)
-		{
-			portal->resowner = NULL;
-			continue;
-		}
-
-		/*
-		 * Do nothing else to cursors held over from a previous
-		 * transaction.
-		 */
-		if (portal->createSubid == InvalidSubTransactionId)
-			continue;
-
+		/* Is it a holdable portal created in the current xact? */
 		if ((portal->cursorOptions & CURSOR_OPT_HOLD) &&
+			portal->createSubid != InvalidSubTransactionId &&
 			portal->status == PORTAL_READY)
 		{
 			/*
@@ -484,12 +462,60 @@ AtCommit_Portals(void)
 			 * as not belonging to this transaction.
 			 */
 			portal->createSubid = InvalidSubTransactionId;
+
+			result = true;
 		}
-		else
+	}
+
+	return result;
+}
+
+/*
+ * Pre-commit processing for portals.
+ *
+ * Remove all non-holdable portals created in this transaction.
+ * Portals remaining from prior transactions should be left untouched.
+ *
+ * XXX This assumes that portals can be deleted in a random order, ie,
+ * no portal has a reference to any other (at least not one that will be
+ * exercised during deletion).	I think this is okay at the moment, but
+ * we've had bugs of that ilk in the past.  Keep a close eye on cursor
+ * references...
+ */
+void
+AtCommit_Portals(void)
+{
+	HASH_SEQ_STATUS status;
+	PortalHashEnt *hentry;
+
+	hash_seq_init(&status, PortalHashTable);
+
+	while ((hentry = (PortalHashEnt *) hash_seq_search(&status)) != NULL)
+	{
+		Portal		portal = hentry->portal;
+
+		/*
+		 * Do not touch active portals --- this can only happen in the
+		 * case of a multi-transaction utility command, such as VACUUM.
+		 *
+		 * Note however that any resource owner attached to such a portal is
+		 * still going to go away, so don't leave a dangling pointer.
+		 */
+		if (portal->status == PORTAL_ACTIVE)
 		{
-			/* Zap all non-holdable portals */
-			PortalDrop(portal, true);
+			portal->resowner = NULL;
+			continue;
 		}
+
+		/*
+		 * Do nothing to cursors held over from a previous transaction
+		 * (including holdable ones just frozen by CommitHoldablePortals).
+		 */
+		if (portal->createSubid == InvalidSubTransactionId)
+			continue;
+
+		/* Zap all non-holdable portals */
+		PortalDrop(portal, true);
 	}
 }