From c1f3073333d01987ac9c3e5f6c197b9e2afc3ba9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 30 Nov 2008 20:51:25 +0000 Subject: [PATCH] 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 | 50 ++++++++++++++++++++++------------- src/backend/commands/copy.c | 4 +-- src/backend/commands/portalcmds.c | 8 ++++-- src/backend/executor/execMain.c | 10 +++---- src/backend/executor/functions.c | 10 +++---- src/backend/executor/spi.c | 9 +++---- src/backend/executor/tstoreReceiver.c | 25 +++++++++++++----- src/backend/tcop/dest.c | 19 +++---------- src/backend/tcop/postgres.c | 10 ++++--- src/backend/tcop/pquery.c | 8 ++++-- src/include/access/printtup.h | 8 +++--- src/include/executor/tstoreReceiver.h | 9 ++++--- src/include/tcop/dest.h | 14 +++++----- src/include/utils/portal.h | 7 ++--- 14 files changed, 108 insertions(+), 83 deletions(-) diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c index 346f6e1d8a..3645fd0f2f 100644 --- 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 2c680008a7..5c4d06812a 100644 --- 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 1d33b78cde..45776277d8 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.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 f63ea4e9eb..cb83e86e7b 100644 --- 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 988f704ad7..b0373da025 100644 --- 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 9cf119eaed..af475892e4 100644 --- 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 2b3889615b..cb78c80b91 100644 --- 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 e687c52f80..ba19094af8 100644 --- 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 c9e7b5e626..f262b8014e 100644 --- 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 f382c73721..2a9764ac8b 100644 --- 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 4f2fcbf41d..3249162ba8 100644 --- 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 d4ea0b2b1d..c304c1fce9 100644 --- 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 5522f78c68..508b7ab378 100644 --- 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 c420d4d71e..c6ccbf493e 100644 --- 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 { -- 2.11.0