AfterTriggerEndSubXact(false);
AtSubAbort_Portals(s->subTransactionId,
s->parent->subTransactionId,
+ s->curTransactionOwner,
s->parent->curTransactionOwner);
AtEOSubXact_LargeObject(false, s->subTransactionId,
s->parent->subTransactionId);
/*
* Check for improper portal use, and mark portal active.
*/
- if (portal->status != PORTAL_READY)
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("portal \"%s\" cannot be run", portal->name)));
- portal->status = PORTAL_ACTIVE;
+ MarkPortalActive(portal);
/*
* Set up global portal context pointers.
/*
* Check for improper portal use, and mark portal active.
*/
- if (portal->status != PORTAL_READY)
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("portal \"%s\" cannot be run", portal->name)));
- portal->status = PORTAL_ACTIVE;
+ MarkPortalActive(portal);
/*
* Set up global portal context pointers.
/*
* Check for improper portal use, and mark portal active.
*/
- if (portal->status != PORTAL_READY)
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("portal \"%s\" cannot be run", portal->name)));
- portal->status = PORTAL_ACTIVE;
+ MarkPortalActive(portal);
/*
* Set up global portal context pointers.
{
/*
* As per notes above, a rel to be rebuilt MUST have refcnt > 0; while of
- * course it would be a bad idea to blow away one with nonzero refcnt.
+ * course it would be an equally bad idea to blow away one with nonzero
+ * refcnt, since that would leave someone somewhere with a dangling
+ * pointer. All callers are expected to have verified that this holds.
*/
Assert(rebuild ?
!RelationHasReferenceCountZero(relation) :
{
if (isCommit)
relation->rd_createSubid = InvalidSubTransactionId;
- else
+ else if (RelationHasReferenceCountZero(relation))
{
RelationClearRelation(relation, false);
return;
}
+ else
+ {
+ /*
+ * Hmm, somewhere there's a (leaked?) reference to the relation.
+ * We daren't remove the entry for fear of dereferencing a
+ * dangling pointer later. Bleat, and mark it as not belonging to
+ * the current transaction. Hopefully it'll get cleaned up
+ * eventually. This must be just a WARNING to avoid
+ * error-during-error-recovery loops.
+ */
+ relation->rd_createSubid = InvalidSubTransactionId;
+ elog(WARNING, "cannot remove relcache entry for \"%s\" because it has nonzero refcount",
+ RelationGetRelationName(relation));
+ }
}
/*
{
if (isCommit)
relation->rd_createSubid = parentSubid;
- else
+ else if (RelationHasReferenceCountZero(relation))
{
RelationClearRelation(relation, false);
return;
}
+ else
+ {
+ /*
+ * Hmm, somewhere there's a (leaked?) reference to the relation.
+ * We daren't remove the entry for fear of dereferencing a
+ * dangling pointer later. Bleat, and transfer it to the parent
+ * subtransaction so we can try again later. This must be just a
+ * WARNING to avoid error-during-error-recovery loops.
+ */
+ relation->rd_createSubid = parentSubid;
+ elog(WARNING, "cannot remove relcache entry for \"%s\" because it has nonzero refcount",
+ RelationGetRelationName(relation));
+ }
}
/*
portal->status = PORTAL_NEW;
portal->cleanup = PortalCleanup;
portal->createSubid = GetCurrentSubTransactionId();
+ portal->activeSubid = portal->createSubid;
portal->strategy = PORTAL_MULTI_QUERY;
portal->cursorOptions = CURSOR_OPT_NO_SCROLL;
portal->atStart = true;
portal->portalPinned = false;
}
+/*
+ * MarkPortalActive
+ * Transition a portal from READY to ACTIVE state.
+ *
+ * NOTE: never set portal->status = PORTAL_ACTIVE directly; call this instead.
+ */
+void
+MarkPortalActive(Portal portal)
+{
+ /* For safety, this is a runtime test not just an Assert */
+ if (portal->status != PORTAL_READY)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("portal \"%s\" cannot be run", portal->name)));
+ /* Perform the state transition */
+ portal->status = PORTAL_ACTIVE;
+ portal->activeSubid = GetCurrentSubTransactionId();
+}
+
/*
* MarkPortalDone
* Transition a portal from ACTIVE to DONE state.
* not belonging to this transaction.
*/
portal->createSubid = InvalidSubTransactionId;
+ portal->activeSubid = InvalidSubTransactionId;
/* Report we changed state */
result = true;
/*
* Pre-subcommit processing for portals.
*
- * Reassign the portals created in the current subtransaction to the parent
- * subtransaction.
+ * Reassign portals created or used in the current subtransaction to the
+ * parent subtransaction.
*/
void
AtSubCommit_Portals(SubTransactionId mySubid,
if (portal->resowner)
ResourceOwnerNewParent(portal->resowner, parentXactOwner);
}
+ if (portal->activeSubid == mySubid)
+ portal->activeSubid = parentSubid;
}
}
/*
* Subtransaction abort handling for portals.
*
- * Deactivate portals created during the failed subtransaction.
- * Note that per AtSubCommit_Portals, this will catch portals created
+ * Deactivate portals created or used during the failed subtransaction.
+ * Note that per AtSubCommit_Portals, this will catch portals created/used
* in descendants of the subtransaction too.
*
* We don't destroy any portals here; that's done in AtSubCleanup_Portals.
void
AtSubAbort_Portals(SubTransactionId mySubid,
SubTransactionId parentSubid,
+ ResourceOwner myXactOwner,
ResourceOwner parentXactOwner)
{
HASH_SEQ_STATUS status;
{
Portal portal = hentry->portal;
+ /* Was it created in this subtransaction? */
if (portal->createSubid != mySubid)
+ {
+ /* No, but maybe it was used in this subtransaction? */
+ if (portal->activeSubid == mySubid)
+ {
+ /* Maintain activeSubid until the portal is removed */
+ 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.
+ *
+ * We assume we can get away without forcing upper-level READY
+ * portals to fail, even if they were run and then suspended.
+ * In theory a suspended upper-level portal could have
+ * acquired some references to objects that are about to be
+ * destroyed, but there should be sufficient defenses against
+ * such cases: the portal's original query cannot contain such
+ * references, and any references within, say, cached plans of
+ * PL/pgSQL functions are not from active queries and should
+ * be protected by revalidation logic.
+ */
+ if (portal->status == PORTAL_ACTIVE)
+ MarkPortalFailed(portal);
+
+ /*
+ * Also, if we failed it during the current subtransaction
+ * (either just above, or earlier), reattach its resource
+ * owner to the current subtransaction's resource owner, so
+ * that any resources it still holds will be released while
+ * cleaning up this subtransaction. This prevents some corner
+ * cases wherein we might get Asserts or worse while cleaning
+ * up objects created during the current subtransaction
+ * (because they're still referenced within this portal).
+ */
+ if (portal->status == PORTAL_FAILED && portal->resowner)
+ {
+ ResourceOwnerNewParent(portal->resowner, myXactOwner);
+ portal->resowner = NULL;
+ }
+ }
+ /* Done if it wasn't created in this subtransaction */
continue;
+ }
/*
* Force any live portals of my own subtransaction into FAILED state.
* We have to do this because they might refer to objects created or
- * changed in the failed subtransaction, leading to crashes if
- * execution is resumed, or even if we just try to run ExecutorEnd.
- * (Note we do NOT do this to upper-level portals, since they cannot
- * have such references and hence may be able to continue.)
+ * changed in the failed subtransaction, leading to crashes within
+ * ExecutorEnd when portalcmds.c tries to close down the portal.
*/
if (portal->status == PORTAL_READY ||
portal->status == PORTAL_ACTIVE)
MemoryContext heap; /* subsidiary memory for portal */
ResourceOwner resowner; /* resources owned by portal */
void (*cleanup) (Portal portal); /* cleanup hook */
- SubTransactionId createSubid; /* the ID of the creating subxact */
/*
- * if createSubid is InvalidSubTransactionId, the portal is held over from
- * a previous transaction
+ * State data for remembering which subtransaction(s) the portal was
+ * created or used in. If the portal is held over from a previous
+ * transaction, both subxids are InvalidSubTransactionId. Otherwise,
+ * createSubid is the creating subxact and activeSubid is the last subxact
+ * in which we ran the portal.
*/
+ SubTransactionId createSubid; /* the creating subxact */
/* The query or queries the portal will execute */
const char *sourceText; /* text of query (as of 8.4, never NULL) */
/* Presentation data, primarily used by the pg_cursors system view */
TimestampTz creation_time; /* time at which this portal was defined */
bool visible; /* include this portal in pg_cursors? */
+
+ /*
+ * This field belongs with createSubid, but in pre-9.5 branches, add it
+ * at the end to avoid creating an ABI break for extensions that examine
+ * Portal structs.
+ */
+ SubTransactionId activeSubid; /* the last subxact with activity */
} PortalData;
/*
ResourceOwner parentXactOwner);
extern void AtSubAbort_Portals(SubTransactionId mySubid,
SubTransactionId parentSubid,
+ ResourceOwner myXactOwner,
ResourceOwner parentXactOwner);
extern void AtSubCleanup_Portals(SubTransactionId mySubid);
extern Portal CreatePortal(const char *name, bool allowDup, bool dupSilent);
extern Portal CreateNewPortal(void);
extern void PinPortal(Portal portal);
extern void UnpinPortal(Portal portal);
+extern void MarkPortalActive(Portal portal);
extern void MarkPortalDone(Portal portal);
extern void MarkPortalFailed(Portal portal);
extern void PortalDrop(Portal portal, bool isTopCommit);
(1 row)
abort;
+-- Test for proper cleanup after a failure in a cursor portal
+-- that was created in an outer subtransaction
+CREATE FUNCTION invert(x float8) RETURNS float8 LANGUAGE plpgsql AS
+$$ begin return 1/x; end $$;
+CREATE FUNCTION create_temp_tab() RETURNS text
+LANGUAGE plpgsql AS $$
+BEGIN
+ CREATE TEMP TABLE new_table (f1 float8);
+ -- case of interest is that we fail while holding an open
+ -- relcache reference to new_table
+ INSERT INTO new_table SELECT invert(0.0);
+ RETURN 'foo';
+END $$;
+BEGIN;
+DECLARE ok CURSOR FOR SELECT * FROM int8_tbl;
+DECLARE ctt CURSOR FOR SELECT create_temp_tab();
+FETCH ok;
+ q1 | q2
+-----+-----
+ 123 | 456
+(1 row)
+
+SAVEPOINT s1;
+FETCH ok; -- should work
+ q1 | q2
+-----+------------------
+ 123 | 4567890123456789
+(1 row)
+
+FETCH ctt; -- error occurs here
+ERROR: division by zero
+CONTEXT: PL/pgSQL function invert(double precision) line 1 at RETURN
+SQL statement "INSERT INTO new_table SELECT invert(0.0)"
+PL/pgSQL function create_temp_tab() line 6 at SQL statement
+ROLLBACK TO s1;
+FETCH ok; -- should work
+ q1 | q2
+------------------+-----
+ 4567890123456789 | 123
+(1 row)
+
+FETCH ctt; -- must be rejected
+ERROR: portal "ctt" cannot be run
+COMMIT;
+DROP FUNCTION create_temp_tab();
+DROP FUNCTION invert(x float8);
-- Test for successful cleanup of an aborted transaction at session exit.
-- THIS MUST BE THE LAST TEST IN THIS FILE.
begin;
abort;
+
+-- Test for proper cleanup after a failure in a cursor portal
+-- that was created in an outer subtransaction
+CREATE FUNCTION invert(x float8) RETURNS float8 LANGUAGE plpgsql AS
+$$ begin return 1/x; end $$;
+
+CREATE FUNCTION create_temp_tab() RETURNS text
+LANGUAGE plpgsql AS $$
+BEGIN
+ CREATE TEMP TABLE new_table (f1 float8);
+ -- case of interest is that we fail while holding an open
+ -- relcache reference to new_table
+ INSERT INTO new_table SELECT invert(0.0);
+ RETURN 'foo';
+END $$;
+
+BEGIN;
+DECLARE ok CURSOR FOR SELECT * FROM int8_tbl;
+DECLARE ctt CURSOR FOR SELECT create_temp_tab();
+FETCH ok;
+SAVEPOINT s1;
+FETCH ok; -- should work
+FETCH ctt; -- error occurs here
+ROLLBACK TO s1;
+FETCH ok; -- should work
+FETCH ctt; -- must be rejected
+COMMIT;
+
+DROP FUNCTION create_temp_tab();
+DROP FUNCTION invert(x float8);
+
+
-- Test for successful cleanup of an aborted transaction at session exit.
-- THIS MUST BE THE LAST TEST IN THIS FILE.
projects / postgresql.git / commitdiff