Clean up the API for DestReceiver objects by eliminating the assumption

that a Portal is a useful and sufficient additional argument for
CreateDestReceiver --- it just isn't, in most cases.  Instead formalize
the approach of passing any needed parameters to the receiver separately.

One unexpected benefit of this change is that we can declare typedef Portal
in a less surprising location.

This patch is just code rearrangement and doesn't change any functionality.
I'll tackle the HOLD-cursor-vs-toast problem in a follow-on patch.

src/backend/access/common/printtup.c

@@ -9,7 +9,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/common/printtup.c,v 1.102 2008/04/17 21:37:28 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/common/printtup.c,v 1.103 2008/11/30 20:51:24 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -67,31 +67,16 @@ typedef struct
  * ----------------
  */
 DestReceiver *
-printtup_create_DR(CommandDest dest, Portal portal)
+printtup_create_DR(CommandDest dest)
 {
-	DR_printtup *self = (DR_printtup *) palloc(sizeof(DR_printtup));
+	DR_printtup *self = (DR_printtup *) palloc0(sizeof(DR_printtup));
 
-	if (PG_PROTOCOL_MAJOR(FrontendProtocol) >= 3)
-		self->pub.receiveSlot = printtup;
-	else
-	{
-		/*
-		 * In protocol 2.0 the Bind message does not exist, so there is no way
-		 * for the columns to have different print formats; it's sufficient to
-		 * look at the first one.
-		 */
-		if (portal->formats && portal->formats[0] != 0)
-			self->pub.receiveSlot = printtup_internal_20;
-		else
-			self->pub.receiveSlot = printtup_20;
-	}
+	self->pub.receiveSlot = printtup;			/* might get changed later */
 	self->pub.rStartup = printtup_startup;
 	self->pub.rShutdown = printtup_shutdown;
 	self->pub.rDestroy = printtup_destroy;
 	self->pub.mydest = dest;
 
-	self->portal = portal;
-
 	/*
 	 * Send T message automatically if DestRemote, but not if
 	 * DestRemoteExecute
@@ -105,6 +90,33 @@ printtup_create_DR(CommandDest dest, Portal portal)
 	return (DestReceiver *) self;
 }
 
+/*
+ * Set parameters for a DestRemote (or DestRemoteExecute) receiver
+ */
+void
+SetRemoteDestReceiverParams(DestReceiver *self, Portal portal)
+{
+	DR_printtup *myState = (DR_printtup *) self;
+
+	Assert(myState->pub.mydest == DestRemote ||
+		   myState->pub.mydest == DestRemoteExecute);
+
+	myState->portal = portal;
+
+	if (PG_PROTOCOL_MAJOR(FrontendProtocol) < 3)
+	{
+		/*
+		 * In protocol 2.0 the Bind message does not exist, so there is no way
+		 * for the columns to have different print formats; it's sufficient to
+		 * look at the first one.
+		 */
+		if (portal->formats && portal->formats[0] != 0)
+			myState->pub.receiveSlot = printtup_internal_20;
+		else
+			myState->pub.receiveSlot = printtup_20;
+	}
+}
+
 static void
 printtup_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
 {

src/backend/commands/copy.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.301 2008/11/06 20:51:14 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.302 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1050,7 +1050,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
 		PushUpdatedSnapshot(GetActiveSnapshot());
 
 		/* Create dest receiver for COPY OUT */
-		dest = CreateDestReceiver(DestCopyOut, NULL);
+		dest = CreateDestReceiver(DestCopyOut);
 		((DR_copy *) dest)->cstate = cstate;
 
 		/* Create a QueryDesc requesting no output */

src/backend/commands/portalcmds.c

@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.75 2008/07/18 20:26:06 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.76 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -26,6 +26,7 @@
 #include "access/xact.h"
 #include "commands/portalcmds.h"
 #include "executor/executor.h"
+#include "executor/tstoreReceiver.h"
 #include "tcop/pquery.h"
 #include "utils/memutils.h"
 #include "utils/snapmgr.h"
@@ -351,7 +352,10 @@ PersistHoldablePortal(Portal portal)
 		ExecutorRewind(queryDesc);
 
 		/* Change the destination to output to the tuplestore */
-		queryDesc->dest = CreateDestReceiver(DestTuplestore, portal);
+		queryDesc->dest = CreateDestReceiver(DestTuplestore);
+		SetTuplestoreDestReceiverParams(queryDesc->dest,
+										portal->holdStore,
+										portal->holdContext);
 
 		/* Fetch the result set into the tuplestore */
 		ExecutorRun(queryDesc, ForwardScanDirection, 0L);

src/backend/executor/execMain.c

@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.318 2008/11/19 01:10:23 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.319 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2833,7 +2833,7 @@ OpenIntoRel(QueryDesc *queryDesc)
 	/*
 	 * Now replace the query's DestReceiver with one for SELECT INTO
 	 */
-	queryDesc->dest = CreateDestReceiver(DestIntoRel, NULL);
+	queryDesc->dest = CreateDestReceiver(DestIntoRel);
 	myState = (DR_intorel *) queryDesc->dest;
 	Assert(myState->pub.mydest == DestIntoRel);
 	myState->estate = estate;
@@ -2877,10 +2877,6 @@ CloseIntoRel(QueryDesc *queryDesc)
 
 /*
  * CreateIntoRelDestReceiver -- create a suitable DestReceiver object
- *
- * Since CreateDestReceiver doesn't accept the parameters we'd need,
- * we just leave the private fields zeroed here.  OpenIntoRel will
- * fill them in.
  */
 DestReceiver *
 CreateIntoRelDestReceiver(void)
@@ -2893,6 +2889,8 @@ CreateIntoRelDestReceiver(void)
 	self->pub.rDestroy = intorel_destroy;
 	self->pub.mydest = DestIntoRel;
 
+	/* private fields will be set by OpenIntoRel */
+
 	return (DestReceiver *) self;
 }
 

src/backend/executor/functions.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.129 2008/11/27 00:10:04 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.130 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -398,7 +398,7 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache)
 	{
 		DR_sqlfunction *myState;
 
-		dest = CreateDestReceiver(DestSQLFunction, NULL);
+		dest = CreateDestReceiver(DestSQLFunction);
 		/* pass down the needed info to the dest receiver routines */
 		myState = (DR_sqlfunction *) dest;
 		Assert(myState->pub.mydest == DestSQLFunction);
@@ -1269,10 +1269,6 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
 
 /*
  * CreateSQLFunctionDestReceiver -- create a suitable DestReceiver object
- *
- * Since CreateDestReceiver doesn't accept the parameters we'd need,
- * we just leave the private fields zeroed here.  postquel_start will
- * fill them in.
  */
 DestReceiver *
 CreateSQLFunctionDestReceiver(void)
@@ -1285,6 +1281,8 @@ CreateSQLFunctionDestReceiver(void)
 	self->pub.rDestroy = sqlfunction_destroy;
 	self->pub.mydest = DestSQLFunction;
 
+	/* private fields will be set by postquel_start */
+
 	return (DestReceiver *) self;
 }
 

src/backend/executor/spi.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.200 2008/11/02 01:45:28 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.201 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1227,7 +1227,7 @@ SPI_cursor_fetch(Portal portal, bool forward, long count)
 {
 	_SPI_cursor_operation(portal,
 						  forward ? FETCH_FORWARD : FETCH_BACKWARD, count,
-						  CreateDestReceiver(DestSPI, NULL));
+						  CreateDestReceiver(DestSPI));
 	/* we know that the DestSPI receiver doesn't need a destroy call */
 }
 
@@ -1256,7 +1256,7 @@ SPI_scroll_cursor_fetch(Portal portal, FetchDirection direction, long count)
 {
 	_SPI_cursor_operation(portal,
 						  direction, count,
-						  CreateDestReceiver(DestSPI, NULL));
+						  CreateDestReceiver(DestSPI));
 	/* we know that the DestSPI receiver doesn't need a destroy call */
 }
 
@@ -1744,8 +1744,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
 			if (!read_only)
 				CommandCounterIncrement();
 
-			dest = CreateDestReceiver(canSetTag ? DestSPI : DestNone,
-									  NULL);
+			dest = CreateDestReceiver(canSetTag ? DestSPI : DestNone);
 
 			if (snapshot == InvalidSnapshot)
 			{

src/backend/executor/tstoreReceiver.c

@@ -9,7 +9,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/tstoreReceiver.c,v 1.19 2008/01/01 19:45:49 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/tstoreReceiver.c,v 1.20 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -72,10 +72,9 @@ tstoreDestroyReceiver(DestReceiver *self)
  * Initially create a DestReceiver object.
  */
 DestReceiver *
-CreateTuplestoreDestReceiver(Tuplestorestate *tStore,
-							 MemoryContext tContext)
+CreateTuplestoreDestReceiver(void)
 {
-	TStoreState *self = (TStoreState *) palloc(sizeof(TStoreState));
+	TStoreState *self = (TStoreState *) palloc0(sizeof(TStoreState));
 
 	self->pub.receiveSlot = tstoreReceiveSlot;
 	self->pub.rStartup = tstoreStartupReceiver;
@@ -83,8 +82,22 @@ CreateTuplestoreDestReceiver(Tuplestorestate *tStore,
 	self->pub.rDestroy = tstoreDestroyReceiver;
 	self->pub.mydest = DestTuplestore;
 
-	self->tstore = tStore;
-	self->cxt = tContext;
+	/* private fields will be set by SetTuplestoreDestReceiverParams */
 
 	return (DestReceiver *) self;
 }
+
+/*
+ * Set parameters for a TuplestoreDestReceiver
+ */
+void
+SetTuplestoreDestReceiverParams(DestReceiver *self,
+								Tuplestorestate *tStore,
+								MemoryContext tContext)
+{
+	TStoreState *myState = (TStoreState *) self;
+
+	Assert(myState->pub.mydest == DestTuplestore);
+	myState->tstore = tStore;
+	myState->cxt = tContext;
+}

src/backend/tcop/dest.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/dest.c,v 1.73 2008/10/31 19:37:56 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/dest.c,v 1.74 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -94,21 +94,16 @@ BeginCommand(const char *commandTag, CommandDest dest)
 
 /* ----------------
  *		CreateDestReceiver - return appropriate receiver function set for dest
- *
- * Note: a Portal must be specified for destinations DestRemote,
- * DestRemoteExecute, and DestTuplestore.  It can be NULL for the others.
  * ----------------
  */
 DestReceiver *
-CreateDestReceiver(CommandDest dest, Portal portal)
+CreateDestReceiver(CommandDest dest)
 {
 	switch (dest)
 	{
 		case DestRemote:
 		case DestRemoteExecute:
-			if (portal == NULL)
-				elog(ERROR, "no portal specified for DestRemote receiver");
-			return printtup_create_DR(dest, portal);
+			return printtup_create_DR(dest);
 
 		case DestNone:
 			return &donothingDR;
@@ -120,13 +115,7 @@ CreateDestReceiver(CommandDest dest, Portal portal)
 			return &spi_printtupDR;
 
 		case DestTuplestore:
-			if (portal == NULL)
-				elog(ERROR, "no portal specified for DestTuplestore receiver");
-			if (portal->holdStore == NULL ||
-				portal->holdContext == NULL)
-				elog(ERROR, "portal has no holdStore");
-			return CreateTuplestoreDestReceiver(portal->holdStore,
-												portal->holdContext);
+			return CreateTuplestoreDestReceiver();
 
 		case DestIntoRel:
 			return CreateIntoRelDestReceiver();

src/backend/tcop/postgres.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.557 2008/09/30 10:52:13 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.558 2008/11/30 20:51:25 tgl Exp $
  *
  * NOTES
  *	  this is the "main" module of the postgres backend and
@@ -979,7 +979,9 @@ exec_simple_query(const char *query_string)
 		/*
 		 * Now we can create the destination receiver object.
 		 */
-		receiver = CreateDestReceiver(dest, portal);
+		receiver = CreateDestReceiver(dest);
+		if (dest == DestRemote)
+			SetRemoteDestReceiverParams(receiver, portal);
 
 		/*
 		 * Switch back to transaction context for execution.
@@ -1835,7 +1837,9 @@ exec_execute_message(const char *portal_name, long max_rows)
 	 * Create dest receiver in MessageContext (we don't want it in transaction
 	 * context, because that may get deleted if portal contains VACUUM).
 	 */
-	receiver = CreateDestReceiver(dest, portal);
+	receiver = CreateDestReceiver(dest);
+	if (dest == DestRemoteExecute)
+		SetRemoteDestReceiverParams(receiver, portal);
 
 	/*
 	 * Ensure we are in a transaction command (this should normally be the

src/backend/tcop/pquery.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.125 2008/11/19 01:10:23 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.126 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,6 +18,7 @@
 #include "access/xact.h"
 #include "commands/prepare.h"
 #include "commands/trigger.h"
+#include "executor/tstoreReceiver.h"
 #include "miscadmin.h"
 #include "pg_trace.h"
 #include "tcop/pquery.h"
@@ -1032,7 +1033,10 @@ FillPortalStore(Portal portal, bool isTopLevel)
 	char		completionTag[COMPLETION_TAG_BUFSIZE];
 
 	PortalCreateHoldStore(portal);
-	treceiver = CreateDestReceiver(DestTuplestore, portal);
+	treceiver = CreateDestReceiver(DestTuplestore);
+	SetTuplestoreDestReceiverParams(treceiver,
+									portal->holdStore,
+									portal->holdContext);
 
 	completionTag[0] = '\0';
 

src/include/access/printtup.h

@@ -7,16 +7,18 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/printtup.h,v 1.36 2008/01/01 19:45:56 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/access/printtup.h,v 1.37 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #ifndef PRINTTUP_H
 #define PRINTTUP_H
 
-#include "tcop/dest.h"
+#include "utils/portal.h"
 
-extern DestReceiver *printtup_create_DR(CommandDest dest, Portal portal);
+extern DestReceiver *printtup_create_DR(CommandDest dest);
+
+extern void SetRemoteDestReceiverParams(DestReceiver *self, Portal portal);
 
 extern void SendRowDescriptionMessage(TupleDesc typeinfo, List *targetlist,
 						  int16 *formats);

src/include/executor/tstoreReceiver.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/executor/tstoreReceiver.h,v 1.10 2008/01/01 19:45:57 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/executor/tstoreReceiver.h,v 1.11 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,7 +19,10 @@
 #include "utils/tuplestore.h"
 
 
-extern DestReceiver *CreateTuplestoreDestReceiver(Tuplestorestate *tStore,
-							 MemoryContext tContext);
+extern DestReceiver *CreateTuplestoreDestReceiver(void);
+
+extern void SetTuplestoreDestReceiverParams(DestReceiver *self,
+											Tuplestorestate *tStore,
+											MemoryContext tContext);
 
 #endif   /* TSTORE_RECEIVER_H */

src/include/tcop/dest.h

@@ -35,6 +35,12 @@
  * The same receiver object may be re-used multiple times; eventually it is
  * destroyed by calling its rDestroy method.
  *
+ * In some cases, receiver objects require additional parameters that must
+ * be passed to them after calling CreateDestReceiver.  Since the set of
+ * parameters varies for different receiver types, this is not handled by
+ * this module, but by direct calls from the calling code to receiver type
+ * specific functions.
+ *
  * The DestReceiver object returned by CreateDestReceiver may be a statically
  * allocated object (for destination types that require no local state),
  * in which case rDestroy is a no-op.  Alternatively it can be a palloc'd
@@ -54,7 +60,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/tcop/dest.h,v 1.55 2008/10/31 19:37:56 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/tcop/dest.h,v 1.56 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -120,14 +126,10 @@ struct _DestReceiver
 
 extern DestReceiver *None_Receiver;		/* permanent receiver for DestNone */
 
-/* This is a forward reference to utils/portal.h */
-
-typedef struct PortalData *Portal;
-
 /* The primary destination management functions */
 
 extern void BeginCommand(const char *commandTag, CommandDest dest);
-extern DestReceiver *CreateDestReceiver(CommandDest dest, Portal portal);
+extern DestReceiver *CreateDestReceiver(CommandDest dest);
 extern void EndCommand(const char *commandTag, CommandDest dest);
 
 /* Additional functions that go with destination management, more or less. */

src/include/utils/portal.h

@@ -39,7 +39,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.79 2008/07/18 20:26:06 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.80 2008/11/30 20:51:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -102,10 +102,7 @@ typedef enum PortalStatus
 	PORTAL_FAILED				/* portal got error (can't re-run it) */
 } PortalStatus;
 
-/*
- * Note: typedef Portal is declared in tcop/dest.h as
- *		typedef struct PortalData *Portal;
- */
+typedef struct PortalData *Portal;
 
 typedef struct PortalData
 {