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.

Author: tglsfdc

src/backend/bootstrap/bootstrap.c

@@ -8,15 +8,14 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/bootstrap/bootstrap.c,v 1.189 2004/07/21 20:34:45 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/bootstrap/bootstrap.c,v 1.190 2004/07/31 00:45:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
 #include <unistd.h>
 #include <signal.h>
-#include <setjmp.h>
 #ifdef HAVE_GETOPT_H
 #include <getopt.h>
 #endif
@@ -458,15 +457,6 @@ BootstrapMain(int argc, char *argv[])
 	for (i = 0; i < HASHTABLESIZE; ++i)
 		hashtable[i] = NULL;
 
-	/*
-	 * abort processing resumes here (this is probably dead code?)
-	 */
-	if (sigsetjmp(Warn_restart, 1) != 0)
-	{
-		Warnings++;
-		AbortCurrentTransaction();
-	}
-
 	/*
 	 * Process bootstrap input.
 	 *

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);
 
@@ -1127,6 +1129,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.
  */

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,91 +327,108 @@ 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;
 
 	ActivePortal = saveActivePortal;
 	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));
 }