From 5fa6b0d102eb8ccd15c4963ee9841baec50df45e Mon Sep 17 00:00:00 2001
From: Tom Lane
Date: Wed, 11 Oct 2017 17:43:50 -0400
Subject: Remove unnecessary PG_TRY overhead for CurrentResourceOwner changes.

resowner/README contained advice to use a PG_TRY block to restore the
old CurrentResourceOwner value anywhere that that variable is transiently
changed.  That advice was only inconsistently followed, however, and
on reflection it seems like unnecessary overhead.  We don't bother
with such a convention for transient CurrentMemoryContext changes,
on the grounds that any (sub)transaction abort will start out by
resetting CurrentMemoryContext to what it wants.  But the same is
true of CurrentResourceOwner, so there seems no need to treat it
differently.

Hence, remove PG_TRY blocks that exist only to restore CurrentResourceOwner
before re-throwing the error.  There are a couple of places that restore
it along with some other actions, and I left those alone; the restore is
probably unnecessary but no noticeable gain will result from removing it.

Discussion: https://postgr.es/m/5236.1507583529@sss.pgh.pa.us
---
 src/backend/commands/portalcmds.c | 22 +++++++---------------
 src/backend/commands/sequence.c   | 16 ++++------------
 src/backend/commands/trigger.c    | 36 +++++++++++-------------------------
 3 files changed, 22 insertions(+), 52 deletions(-)

(limited to 'src/backend/commands')

diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index b36473fba44..76d6cf154c6 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -294,21 +294,13 @@ PortalCleanup(Portal portal)
 
 			/* We must make the portal's resource owner current */
 			saveResourceOwner = CurrentResourceOwner;
-			PG_TRY();
-			{
-				if (portal->resowner)
-					CurrentResourceOwner = portal->resowner;
-				ExecutorFinish(queryDesc);
-				ExecutorEnd(queryDesc);
-				FreeQueryDesc(queryDesc);
-			}
-			PG_CATCH();
-			{
-				/* Ensure CurrentResourceOwner is restored on error */
-				CurrentResourceOwner = saveResourceOwner;
-				PG_RE_THROW();
-			}
-			PG_END_TRY();
+			if (portal->resowner)
+				CurrentResourceOwner = portal->resowner;
+
+			ExecutorFinish(queryDesc);
+			ExecutorEnd(queryDesc);
+			FreeQueryDesc(queryDesc);
+
 			CurrentResourceOwner = saveResourceOwner;
 		}
 	}
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 5c2ce789468..5e1b0fe2897 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1055,18 +1055,10 @@ lock_and_open_sequence(SeqTable seq)
 		ResourceOwner currentOwner;
 
 		currentOwner = CurrentResourceOwner;
-		PG_TRY();
-		{
-			CurrentResourceOwner = TopTransactionResourceOwner;
-			LockRelationOid(seq->relid, RowExclusiveLock);
-		}
-		PG_CATCH();
-		{
-			/* Ensure CurrentResourceOwner is restored on error */
-			CurrentResourceOwner = currentOwner;
-			PG_RE_THROW();
-		}
-		PG_END_TRY();
+		CurrentResourceOwner = TopTransactionResourceOwner;
+
+		LockRelationOid(seq->relid, RowExclusiveLock);
+
 		CurrentResourceOwner = currentOwner;
 
 		/* Flag that we have a lock in the current xact */
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index e75a59d2995..8d0345cd64e 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3639,17 +3639,10 @@ GetCurrentFDWTuplestore(void)
 		 */
 		oldcxt = MemoryContextSwitchTo(CurTransactionContext);
 		saveResourceOwner = CurrentResourceOwner;
-		PG_TRY();
-		{
-			CurrentResourceOwner = CurTransactionResourceOwner;
-			ret = tuplestore_begin_heap(false, false, work_mem);
-		}
-		PG_CATCH();
-		{
-			CurrentResourceOwner = saveResourceOwner;
-			PG_RE_THROW();
-		}
-		PG_END_TRY();
+		CurrentResourceOwner = CurTransactionResourceOwner;
+
+		ret = tuplestore_begin_heap(false, false, work_mem);
+
 		CurrentResourceOwner = saveResourceOwner;
 		MemoryContextSwitchTo(oldcxt);
 
@@ -4468,20 +4461,13 @@ MakeTransitionCaptureState(TriggerDesc *trigdesc, Oid relid, CmdType cmdType)
 	/* Now create required tuplestore(s), if we don't have them already. */
 	oldcxt = MemoryContextSwitchTo(CurTransactionContext);
 	saveResourceOwner = CurrentResourceOwner;
-	PG_TRY();
-	{
-		CurrentResourceOwner = CurTransactionResourceOwner;
-		if (need_old && table->old_tuplestore == NULL)
-			table->old_tuplestore = tuplestore_begin_heap(false, false, work_mem);
-		if (need_new && table->new_tuplestore == NULL)
-			table->new_tuplestore = tuplestore_begin_heap(false, false, work_mem);
-	}
-	PG_CATCH();
-	{
-		CurrentResourceOwner = saveResourceOwner;
-		PG_RE_THROW();
-	}
-	PG_END_TRY();
+	CurrentResourceOwner = CurTransactionResourceOwner;
+
+	if (need_old && table->old_tuplestore == NULL)
+		table->old_tuplestore = tuplestore_begin_heap(false, false, work_mem);
+	if (need_new && table->new_tuplestore == NULL)
+		table->new_tuplestore = tuplestore_begin_heap(false, false, work_mem);
+
 	CurrentResourceOwner = saveResourceOwner;
 	MemoryContextSwitchTo(oldcxt);
 
-- 
cgit v1.2.3