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.

diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index 346f6e1..3645fd0 100644 (file)
--- a/src/backend/access/common/printtup.c
+++ b/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)
 {

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 2c68000..5c4d068 100644 (file)
--- a/src/backend/commands/copy.c
+++ b/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 */

diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 1d33b78..4577627 100644 (file)
--- 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.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);

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index f63ea4e..cb83e86 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/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;
 }
 

diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 988f704..b0373da 100644 (file)
--- a/src/backend/executor/functions.c
+++ b/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;
 }
 

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 9cf119e..af47589 100644 (file)
--- a/src/backend/executor/spi.c
+++ b/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)
                        {

diff --git a/src/backend/executor/tstoreReceiver.c b/src/backend/executor/tstoreReceiver.c
index 2b38896..cb78c80 100644 (file)
--- a/src/backend/executor/tstoreReceiver.c
+++ b/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;
+}

diff --git a/src/backend/tcop/dest.c b/src/backend/tcop/dest.c
index e687c52..ba19094 100644 (file)
--- a/src/backend/tcop/dest.c
+++ b/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();

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index c9e7b5e..f262b80 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/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

diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index f382c73..2a9764a 100644 (file)
--- a/src/backend/tcop/pquery.c
+++ b/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';
 

diff --git a/src/include/access/printtup.h b/src/include/access/printtup.h
index 4f2fcbf..3249162 100644 (file)
--- a/src/include/access/printtup.h
+++ b/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);

diff --git a/src/include/executor/tstoreReceiver.h b/src/include/executor/tstoreReceiver.h
index d4ea0b2..c304c1f 100644 (file)
--- a/src/include/executor/tstoreReceiver.h
+++ b/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 */

diff --git a/src/include/tcop/dest.h b/src/include/tcop/dest.h
index 5522f78..508b7ab 100644 (file)
--- a/src/include/tcop/dest.h
+++ b/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. */

diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index c420d4d..c6ccbf4 100644 (file)
--- a/src/include/utils/portal.h
+++ b/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
 {