Restructure error handling as recently discussed. It is now really

possible to trap an error inside a function rather than letting it
propagate out to PostgresMain.  You still have to use AbortCurrentTransaction
to clean up, but at least the error handling itself will cooperate.

3 files changed, 194 insertions, 121 deletions

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 4a05b40eea..8307e9b47d 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -8,13 +8,12 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.227 2004/06/16 01:26:42 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.228 2004/07/31 00:45:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
-#include <errno.h>
 #include <unistd.h>
 #include <sys/stat.h>
 #include <netinet/in.h>
@@ -130,6 +129,9 @@ static StringInfoData line_buf;
 static bool line_buf_converted;
 
 /* non-export function prototypes */
+static void DoCopyTo(Relation rel, List *attnumlist, bool binary, bool oids,
+		 char *delim, char *null_print, bool csv_mode, char *quote,
+		 char *escape, List *force_quote_atts, bool fe_copy);
 static void CopyTo(Relation rel, List *attnumlist, bool binary, bool oids,
 	   char *delim, char *null_print, bool csv_mode, char *quote, char *escape,
 	   List *force_quote_atts);
@@ -688,6 +690,7 @@ DoCopy(const CopyStmt *stmt)
 	ListCell   *option;
 	List	   *attnamelist = stmt->attlist;
 	List	   *attnumlist;
+	bool		fe_copy = false;
 	bool		binary = false;
 	bool		oids = false;
 	bool        csv_mode = false;
@@ -1062,7 +1065,7 @@ DoCopy(const CopyStmt *stmt)
 		if (pipe)
 		{
 			if (whereToSendOutput == Remote)
-				SendCopyBegin(binary, list_length(attnumlist));
+				fe_copy = true;
 			else
 				copy_file = stdout;
 		}
@@ -1099,8 +1102,9 @@ DoCopy(const CopyStmt *stmt)
 						 errmsg("\"%s\" is a directory", filename)));
 			}
 		}
-		CopyTo(rel, attnumlist, binary, oids, delim, null_print, csv_mode,
-				quote, escape, force_quote_atts);
+
+		DoCopyTo(rel, attnumlist, binary, oids, delim, null_print, csv_mode,
+				 quote, escape, force_quote_atts, fe_copy);
 	}
 
 	if (!pipe)
@@ -1112,8 +1116,6 @@ DoCopy(const CopyStmt *stmt)
 					 errmsg("could not write to file \"%s\": %m",
 							filename)));
 	}
-	else if (whereToSendOutput == Remote && !is_from)
-		SendCopyEnd(binary);
 	pfree(attribute_buf.data);
 	pfree(line_buf.data);
 
@@ -1128,6 +1130,39 @@ DoCopy(const CopyStmt *stmt)
 
 
 /*
+ * This intermediate routine just exists to localize the effects of setjmp
+ * so we don't need to plaster a lot of variables with "volatile".
+ */
+static void
+DoCopyTo(Relation rel, List *attnumlist, bool binary, bool oids,
+		 char *delim, char *null_print, bool csv_mode, char *quote,
+		 char *escape, List *force_quote_atts, bool fe_copy)
+{
+	PG_TRY();
+	{
+		if (fe_copy)
+			SendCopyBegin(binary, list_length(attnumlist));
+
+		CopyTo(rel, attnumlist, binary, oids, delim, null_print, csv_mode,
+			   quote, escape, force_quote_atts);
+
+		if (fe_copy)
+			SendCopyEnd(binary);
+	}
+	PG_CATCH();
+	{
+		/*
+		 * Make sure we turn off old-style COPY OUT mode upon error.
+		 * It is okay to do this in all cases, since it does nothing
+		 * if the mode is not on.
+		 */
+		pq_endcopyout(true);
+		PG_RE_THROW();
+	}
+	PG_END_TRY();
+}
+
+/*
  * Copy from relation TO file.
  */
 static void
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index b176a6c0c7..509d9e0dfa 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.29 2004/07/17 03:28:47 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.30 2004/07/31 00:45:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -259,8 +259,18 @@ PortalCleanup(Portal portal)
 
 			/* We must make the portal's resource owner current */
 			saveResourceOwner = CurrentResourceOwner;
-			CurrentResourceOwner = portal->resowner;
-			ExecutorEnd(queryDesc);
+			PG_TRY();
+			{
+				CurrentResourceOwner = portal->resowner;
+				ExecutorEnd(queryDesc);
+			}
+			PG_CATCH();
+			{
+				/* Ensure CurrentResourceOwner is restored on error */
+				CurrentResourceOwner = saveResourceOwner;
+				PG_RE_THROW();
+			}
+			PG_END_TRY();
 			CurrentResourceOwner = saveResourceOwner;
 		}
 	}
@@ -317,86 +327,95 @@ PersistHoldablePortal(Portal portal)
 	portal->status = PORTAL_ACTIVE;
 
 	/*
-	 * Set global portal context pointers.
+	 * Set up global portal context pointers.
 	 */
 	saveActivePortal = ActivePortal;
-	ActivePortal = portal;
 	saveResourceOwner = CurrentResourceOwner;
-	CurrentResourceOwner = portal->resowner;
 	savePortalContext = PortalContext;
-	PortalContext = PortalGetHeapMemory(portal);
 	saveQueryContext = QueryContext;
-	QueryContext = portal->queryContext;
+	PG_TRY();
+	{
+		ActivePortal = portal;
+		CurrentResourceOwner = portal->resowner;
+		PortalContext = PortalGetHeapMemory(portal);
+		QueryContext = portal->queryContext;
+
+		MemoryContextSwitchTo(PortalContext);
+
+		/*
+		 * Rewind the executor: we need to store the entire result set in the
+		 * tuplestore, so that subsequent backward FETCHs can be processed.
+		 */
+		ExecutorRewind(queryDesc);
+
+		/* Change the destination to output to the tuplestore */
+		queryDesc->dest = CreateDestReceiver(Tuplestore, portal);
+
+		/* Fetch the result set into the tuplestore */
+		ExecutorRun(queryDesc, ForwardScanDirection, 0L);
+
+		(*queryDesc->dest->rDestroy) (queryDesc->dest);
+		queryDesc->dest = NULL;
+
+		/*
+		 * Now shut down the inner executor.
+		 */
+		portal->queryDesc = NULL;	/* prevent double shutdown */
+		ExecutorEnd(queryDesc);
+
+		/*
+		 * Reset the position in the result set: ideally, this could be
+		 * implemented by just skipping straight to the tuple # that we need
+		 * to be at, but the tuplestore API doesn't support that. So we start
+		 * at the beginning of the tuplestore and iterate through it until we
+		 * reach where we need to be.  FIXME someday?
+		 */
+		MemoryContextSwitchTo(portal->holdContext);
+
+		if (!portal->atEnd)
+		{
+			long		store_pos;
 
-	MemoryContextSwitchTo(PortalContext);
+			if (portal->posOverflow)	/* oops, cannot trust portalPos */
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("could not reposition held cursor")));
 
-	/*
-	 * Rewind the executor: we need to store the entire result set in the
-	 * tuplestore, so that subsequent backward FETCHs can be processed.
-	 */
-	ExecutorRewind(queryDesc);
+			tuplestore_rescan(portal->holdStore);
 
-	/* Change the destination to output to the tuplestore */
-	queryDesc->dest = CreateDestReceiver(Tuplestore, portal);
+			for (store_pos = 0; store_pos < portal->portalPos; store_pos++)
+			{
+				HeapTuple	tup;
+				bool		should_free;
 
-	/* Fetch the result set into the tuplestore */
-	ExecutorRun(queryDesc, ForwardScanDirection, 0L);
+				tup = tuplestore_gettuple(portal->holdStore, true,
+										  &should_free);
 
-	(*queryDesc->dest->rDestroy) (queryDesc->dest);
-	queryDesc->dest = NULL;
+				if (tup == NULL)
+					elog(ERROR, "unexpected end of tuple stream");
 
-	/*
-	 * Now shut down the inner executor.
-	 */
-	portal->queryDesc = NULL;	/* prevent double shutdown */
-	ExecutorEnd(queryDesc);
-
-	/*
-	 * Reset the position in the result set: ideally, this could be
-	 * implemented by just skipping straight to the tuple # that we need
-	 * to be at, but the tuplestore API doesn't support that. So we start
-	 * at the beginning of the tuplestore and iterate through it until we
-	 * reach where we need to be.  FIXME someday?
-	 */
-	MemoryContextSwitchTo(portal->holdContext);
-
-	if (!portal->atEnd)
+				if (should_free)
+					pfree(tup);
+			}
+		}
+	}
+	PG_CATCH();
 	{
-		long		store_pos;
-
-		if (portal->posOverflow)	/* oops, cannot trust portalPos */
-			ereport(ERROR,
-					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("could not reposition held cursor")));
-
-		tuplestore_rescan(portal->holdStore);
-
-		for (store_pos = 0; store_pos < portal->portalPos; store_pos++)
-		{
-			HeapTuple	tup;
-			bool		should_free;
-
-			tup = tuplestore_gettuple(portal->holdStore, true,
-									  &should_free);
+		/* Uncaught error while executing portal: mark it dead */
+		portal->status = PORTAL_FAILED;
 
-			if (tup == NULL)
-				elog(ERROR, "unexpected end of tuple stream");
+		/* Restore global vars and propagate error */
+		ActivePortal = saveActivePortal;
+		CurrentResourceOwner = saveResourceOwner;
+		PortalContext = savePortalContext;
+		QueryContext = saveQueryContext;
 
-			if (should_free)
-				pfree(tup);
-		}
+		PG_RE_THROW();
 	}
+	PG_END_TRY();
 
 	MemoryContextSwitchTo(oldcxt);
 
-	/*
-	 * We can now release any subsidiary memory of the portal's heap
-	 * context; we'll never use it again.  The executor already dropped
-	 * its context, but this will clean up anything that glommed onto the
-	 * portal's heap via PortalContext.
-	 */
-	MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
-
 	/* Mark portal not active */
 	portal->status = PORTAL_READY;
 
@@ -404,4 +423,12 @@ PersistHoldablePortal(Portal portal)
 	CurrentResourceOwner = saveResourceOwner;
 	PortalContext = savePortalContext;
 	QueryContext = saveQueryContext;
+
+	/*
+	 * We can now release any subsidiary memory of the portal's heap
+	 * context; we'll never use it again.  The executor already dropped
+	 * its context, but this will clean up anything that glommed onto the
+	 * portal's heap via PortalContext.
+	 */
+	MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
 }
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index dfc03ea461..23d7996cf5 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.284 2004/07/21 22:31:21 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.285 2004/07/31 00:45:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -228,14 +228,13 @@ void
 vacuum(VacuumStmt *vacstmt)
 {
 	const char *stmttype = vacstmt->vacuum ? "VACUUM" : "ANALYZE";
-	MemoryContext anl_context = NULL;
 	TransactionId initialOldestXmin = InvalidTransactionId;
 	TransactionId initialFreezeLimit = InvalidTransactionId;
-	bool		all_rels,
+	volatile MemoryContext anl_context = NULL;
+	volatile bool all_rels,
 				in_outer_xact,
 				use_own_xacts;
 	List	   *relations;
-	ListCell   *cur;
 
 	if (vacstmt->verbose)
 		elevel = INFO;
@@ -267,10 +266,6 @@ vacuum(VacuumStmt *vacstmt)
 		in_outer_xact = IsInTransactionChain((void *) vacstmt);
 	}
 
-	/* Turn vacuum cost accounting on or off */
-	VacuumCostActive = (VacuumCostNaptime > 0);
-	VacuumCostBalance = 0;
-
 	/*
 	 * Send info about dead objects to the statistics collector
 	 */
@@ -377,57 +372,76 @@ vacuum(VacuumStmt *vacstmt)
 		CommitTransactionCommand();
 	}
 
-	/*
-	 * Loop to process each selected relation.
-	 */
-	foreach(cur, relations)
+	/* Turn vacuum cost accounting on or off */
+	PG_TRY();
 	{
-		Oid			relid = lfirst_oid(cur);
+		ListCell   *cur;
 
-		if (vacstmt->vacuum)
-		{
-			if (!vacuum_rel(relid, vacstmt, RELKIND_RELATION))
-				all_rels = false;		/* forget about updating dbstats */
-		}
-		if (vacstmt->analyze)
+		VacuumCostActive = (VacuumCostNaptime > 0);
+		VacuumCostBalance = 0;
+
+		/*
+		 * Loop to process each selected relation.
+		 */
+		foreach(cur, relations)
 		{
-			MemoryContext old_context = NULL;
+			Oid			relid = lfirst_oid(cur);
 
-			/*
-			 * If using separate xacts, start one for analyze. Otherwise,
-			 * we can use the outer transaction, but we still need to call
-			 * analyze_rel in a memory context that will be cleaned up on
-			 * return (else we leak memory while processing multiple
-			 * tables).
-			 */
-			if (use_own_xacts)
+			if (vacstmt->vacuum)
 			{
-				StartTransactionCommand();
-				SetQuerySnapshot();		/* might be needed for functions
-										 * in indexes */
+				if (!vacuum_rel(relid, vacstmt, RELKIND_RELATION))
+					all_rels = false; /* forget about updating dbstats */
 			}
-			else
-				old_context = MemoryContextSwitchTo(anl_context);
+			if (vacstmt->analyze)
+			{
+				MemoryContext old_context = NULL;
 
-			/*
-			 * Tell the buffer replacement strategy that vacuum is
-			 * causing the IO
-			 */
-			StrategyHintVacuum(true);
+				/*
+				 * If using separate xacts, start one for analyze. Otherwise,
+				 * we can use the outer transaction, but we still need to call
+				 * analyze_rel in a memory context that will be cleaned up on
+				 * return (else we leak memory while processing multiple
+				 * tables).
+				 */
+				if (use_own_xacts)
+				{
+					StartTransactionCommand();
+					SetQuerySnapshot();		/* might be needed for functions
+											 * in indexes */
+				}
+				else
+					old_context = MemoryContextSwitchTo(anl_context);
+
+				/*
+				 * Tell the buffer replacement strategy that vacuum is
+				 * causing the IO
+				 */
+				StrategyHintVacuum(true);
 
-			analyze_rel(relid, vacstmt);
+				analyze_rel(relid, vacstmt);
 
-			StrategyHintVacuum(false);
+				StrategyHintVacuum(false);
 
-			if (use_own_xacts)
-				CommitTransactionCommand();
-			else
-			{
-				MemoryContextSwitchTo(old_context);
-				MemoryContextResetAndDeleteChildren(anl_context);
+				if (use_own_xacts)
+					CommitTransactionCommand();
+				else
+				{
+					MemoryContextSwitchTo(old_context);
+					MemoryContextResetAndDeleteChildren(anl_context);
+				}
 			}
 		}
 	}
+	PG_CATCH();
+	{
+		/* Make sure cost accounting is turned off after error */
+		VacuumCostActive = false;
+		PG_RE_THROW();
+	}
+	PG_END_TRY();
+
+	/* Turn off vacuum cost accounting */
+	VacuumCostActive = false;
 
 	/*
 	 * Finish up processing.
@@ -475,9 +489,6 @@ vacuum(VacuumStmt *vacstmt)
 
 	if (anl_context)
 		MemoryContextDelete(anl_context);
-
-	/* Turn off vacuum cost accounting */
-	VacuumCostActive = false;
 }
 
 /*