Comment on dead code in AtAbort_Portals() and AtSubAbort_Portals().

Reviewed by Tom Lane and Robert Haas.

diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index a53673cc2a788602c7dee2ad38d6c7792fd4e296..053b613cb2307bb57126fa169af7ab49fb831d18 100644 (file)
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -765,7 +765,14 @@ AtAbort_Portals(void)
        {
                Portal          portal = hentry->portal;
 
-               /* Any portal that was actually running has to be considered broken */
+               /*
+                * See similar code in AtSubAbort_Portals().  This would fire if code
+                * orchestrating multiple top-level transactions within a portal, such
+                * as VACUUM, caught errors and continued under the same portal with a
+                * fresh transaction.  No part of core PostgreSQL functions that way.
+                * XXX Such code would wish the portal to remain ACTIVE, as in
+                * PreCommit_Portals().
+                */
                if (portal->status == PORTAL_ACTIVE)
                        MarkPortalFailed(portal);
 
@@ -919,9 +926,10 @@ AtSubAbort_Portals(SubTransactionId mySubid,
                                portal->activeSubid = parentSubid;
 
                                /*
-                                * Upper-level portals that failed while running in this
-                                * subtransaction must be forced into FAILED state, for the
-                                * same reasons discussed below.
+                                * A MarkPortalActive() caller ran an upper-level portal in
+                                * this subtransaction and left the portal ACTIVE.  This can't
+                                * happen, but force the portal into FAILED state for the same
+                                * reasons discussed below.
                                 *
                                 * We assume we can get away without forcing upper-level READY
                                 * portals to fail, even if they were run and then suspended.
@@ -961,6 +969,10 @@ AtSubAbort_Portals(SubTransactionId mySubid,
                 * We have to do this because they might refer to objects created or
                 * changed in the failed subtransaction, leading to crashes within
                 * ExecutorEnd when portalcmds.c tries to close down the portal.
+                * Currently, every MarkPortalActive() caller ensures it updates the
+                * portal status again before relinquishing control, so ACTIVE can't
+                * happen here.  If it does happen, dispose the portal like existing
+                * MarkPortalActive() callers would.
                 */
                if (portal->status == PORTAL_READY ||
                        portal->status == PORTAL_ACTIVE)