diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b5bb3d5b1fc40ad8d88c90a1c754e97d4f403313..4d896ef551da662342c340f595d1f4a1d708f21c 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -10,7 +10,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.198 2005/03/28 01:50:33 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.199 2005/04/11 19:51:14 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1439,16 +1439,32 @@ CommitTransaction(void)
/*
* Do pre-commit processing (most of this stuff requires database
* access, and in fact could still cause an error...)
+ *
+ * It is possible for CommitHoldablePortals to invoke functions that
+ * queue deferred triggers, and it's also possible that triggers create
+ * holdable cursors. So we have to loop until there's nothing left to
+ * do.
*/
+ for (;;)
+ {
+ /*
+ * Fire all currently pending deferred triggers.
+ */
+ AfterTriggerFireDeferred();
- /*
- * Tell the trigger manager that this transaction is about to be
- * committed. He'll invoke all trigger deferred until XACT before we
- * really start on committing the transaction.
- */
- AfterTriggerEndXact();
+ /*
+ * Convert any open holdable cursors into static portals. If there
+ * weren't any, we are done ... otherwise loop back to check if they
+ * queued deferred triggers. Lather, rinse, repeat.
+ */
+ if (!CommitHoldablePortals())
+ break;
+ }
+
+ /* Now we can shut down the deferred-trigger manager */
+ AfterTriggerEndXact(true);
- /* Close open cursors */
+ /* Close any open regular cursors */
AtCommit_Portals();
/*
@@ -1649,7 +1665,7 @@ AbortTransaction(void)
/*
* do abort processing
*/
- AfterTriggerAbortXact();
+ AfterTriggerEndXact(false);
AtAbort_Portals();
AtEOXact_LargeObject(false); /* 'false' means it's abort */
AtAbort_Notify();
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index ef00943055fadfffc6ad939efd52af059c56170e..8c8ed9e21caf7527e6b3849e1e7e0faa4860c44f 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.183 2005/03/29 03:01:30 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.184 2005/04/11 19:51:15 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -2441,14 +2441,18 @@ AfterTriggerEndQuery(EState *estate)
/* ----------
- * AfterTriggerEndXact()
+ * AfterTriggerFireDeferred()
*
* Called just before the current transaction is committed. At this
- * time we invoke all DEFERRED triggers and tidy up.
+ * time we invoke all pending DEFERRED triggers.
+ *
+ * It is possible for other modules to queue additional deferred triggers
+ * during pre-commit processing; therefore xact.c may have to call this
+ * multiple times.
* ----------
*/
void
-AfterTriggerEndXact(void)
+AfterTriggerFireDeferred(void)
{
AfterTriggerEventList *events;
@@ -2463,14 +2467,14 @@ AfterTriggerEndXact(void)
* for them to use. (Since PortalRunUtility doesn't set a snap for
* COMMIT, we can't assume ActiveSnapshot is valid on entry.)
*/
- if (afterTriggers->events.head != NULL)
+ events = &afterTriggers->events;
+ if (events->head != NULL)
ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
/*
* Run all the remaining triggers. Loop until they are all gone,
* just in case some trigger queues more for us to do.
*/
- events = &afterTriggers->events;
while (afterTriggerMarkEvents(events, NULL, false))
{
CommandId firing_id = afterTriggers->firing_counter++;
@@ -2478,34 +2482,26 @@ AfterTriggerEndXact(void)
afterTriggerInvokeEvents(events, firing_id, NULL, true);
}
- /*
- * Forget everything we know about AFTER triggers.
- *
- * Since all the info is in TopTransactionContext or children thereof, we
- * need do nothing special to reclaim memory.
- */
- afterTriggers = NULL;
+ Assert(events->head == NULL);
}
/* ----------
- * AfterTriggerAbortXact()
+ * AfterTriggerEndXact()
+ *
+ * The current transaction is finishing.
*
- * The current transaction has entered the abort state.
- * All outstanding triggers are canceled so we simply throw
+ * Any unfired triggers are canceled so we simply throw
* away anything we know.
+ *
+ * Note: it is possible for this to be called repeatedly in case of
+ * error during transaction abort; therefore, do not complain if
+ * already closed down.
* ----------
*/
void
-AfterTriggerAbortXact(void)
+AfterTriggerEndXact(bool isCommit)
{
- /*
- * Ignore call if we aren't in a transaction. (Need this to survive
- * repeat call in case of error during transaction abort.)
- */
- if (afterTriggers == NULL)
- return;
-
/*
* Forget everything we know about AFTER triggers.
*
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 95e1972fe30012ed3c61b77fc1a2760adfa6f441..fd8a737adae0996e2ccb76ce9b1200f6e3ff563c 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.77 2005/01/26 23:20:21 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.78 2005/04/11 19:51:15 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);
}
}
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 1aacccc8110221f4a73fde6fb8a8381816117a08..81a665e24767e5f9b4d8fdecd5d554ddf2355690 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -6,7 +6,7 @@
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.52 2005/03/25 21:57:59 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.53 2005/04/11 19:51:15 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -157,8 +157,8 @@ extern void ExecARUpdateTriggers(EState *estate,
extern void AfterTriggerBeginXact(void);
extern void AfterTriggerBeginQuery(void);
extern void AfterTriggerEndQuery(EState *estate);
-extern void AfterTriggerEndXact(void);
-extern void AfterTriggerAbortXact(void);
+extern void AfterTriggerFireDeferred(void);
+extern void AfterTriggerEndXact(bool isCommit);
extern void AfterTriggerBeginSubXact(void);
extern void AfterTriggerEndSubXact(bool isCommit);
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index 75e606c180ff214cbd0eda3b54b9f8a98a0c2451..b8bcc33f583f6fa152b6ad2ba889914594a2b5a6 100644
--- a/src/include/utils/portal.h
+++ b/src/include/utils/portal.h
@@ -39,7 +39,7 @@
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.54 2004/12/31 22:03:46 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.55 2005/04/11 19:51:16 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -182,6 +182,7 @@ typedef struct PortalData
/* Prototypes for functions in utils/mmgr/portalmem.c */
extern void EnablePortalManager(void);
+extern bool CommitHoldablePortals(void);
extern void AtCommit_Portals(void);
extern void AtAbort_Portals(void);
extern void AtCleanup_Portals(void);
diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index a46a14ce80ce8ce7fc2ea28a9c045f97ebaaf273..3f0e0cdd265fdc1741285e75938c7380746c546f 100644
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -773,3 +773,38 @@ FETCH ALL FROM c;
(15 rows)
ROLLBACK;
+--
+-- Test behavior of both volatile and stable functions inside a cursor;
+-- in particular we want to see what happens during commit of a holdable
+-- cursor
+--
+create temp table tt1(f1 int);
+create function count_tt1_v() returns int8 as
+'select count(*) from tt1' language sql volatile;
+create function count_tt1_s() returns int8 as
+'select count(*) from tt1' language sql stable;
+begin;
+insert into tt1 values(1);
+declare c1 cursor for select count_tt1_v(), count_tt1_s();
+insert into tt1 values(2);
+fetch all from c1;
+ count_tt1_v | count_tt1_s
+-------------+-------------
+ 2 | 1
+(1 row)
+
+rollback;
+begin;
+insert into tt1 values(1);
+declare c2 cursor with hold for select count_tt1_v(), count_tt1_s();
+insert into tt1 values(2);
+commit;
+delete from tt1;
+fetch all from c2;
+ count_tt1_v | count_tt1_s
+-------------+-------------
+ 2 | 1
+(1 row)
+
+drop function count_tt1_v();
+drop function count_tt1_s();
diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql
index c7e29c378680f6dbf53e99841949b49537e04f6e..da4e3b0e3ae591863c2087575e2ff935f0882a76 100644
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -235,3 +235,46 @@ SELECT declares_cursor('AB%');
FETCH ALL FROM c;
ROLLBACK;
+
+--
+-- Test behavior of both volatile and stable functions inside a cursor;
+-- in particular we want to see what happens during commit of a holdable
+-- cursor
+--
+
+create temp table tt1(f1 int);
+
+create function count_tt1_v() returns int8 as
+'select count(*) from tt1' language sql volatile;
+
+create function count_tt1_s() returns int8 as
+'select count(*) from tt1' language sql stable;
+
+begin;
+
+insert into tt1 values(1);
+
+declare c1 cursor for select count_tt1_v(), count_tt1_s();
+
+insert into tt1 values(2);
+
+fetch all from c1;
+
+rollback;
+
+begin;
+
+insert into tt1 values(1);
+
+declare c2 cursor with hold for select count_tt1_v(), count_tt1_s();
+
+insert into tt1 values(2);
+
+commit;
+
+delete from tt1;
+
+fetch all from c2;
+
+drop function count_tt1_v();
+drop function count_tt1_s();