diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c index 6efd09c44b1..5a93aec6890 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.69 2008/01/01 19:45:49 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.69.2.1 2008/04/02 18:32:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -76,6 +76,9 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params, stmt = copyObject(stmt); stmt->utilityStmt = NULL; /* make it look like plain SELECT */ + if (queryString) /* copy the source text too for safety */ + queryString = pstrdup(queryString); + PortalDefineQuery(portal, NULL, queryString, diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index 2199e109d46..7dcd3cb5a85 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -10,7 +10,7 @@ * Copyright (c) 2002-2008, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.80 2008/01/01 19:45:49 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.80.2.1 2008/04/02 18:32:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -249,9 +249,13 @@ ExecuteQuery(ExecuteStmt *stmt, const char *queryString, plan_list = cplan->stmt_list; } + /* + * Note: we don't bother to copy the source query string into the portal. + * Any errors it might be useful for will already have been reported. + */ PortalDefineQuery(portal, NULL, - entry->plansource->query_string, + NULL, entry->plansource->commandTag, plan_list, cplan); diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index a23c4d017a8..f79868630f6 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.188 2008/02/12 04:09:44 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.188.2.1 2008/04/02 18:32:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -858,6 +858,7 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan, CachedPlanSource *plansource; CachedPlan *cplan; List *stmt_list; + char *query_string; ParamListInfo paramLI; Snapshot snapshot; MemoryContext oldcontext; @@ -908,10 +909,22 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan, portal = CreatePortal(name, false, false); } + /* + * Prepare to copy stuff into the portal's memory context. We do all this + * copying first, because it could possibly fail (out-of-memory) and we + * don't want a failure to occur between RevalidateCachedPlan and + * PortalDefineQuery; that would result in leaking our plancache refcount. + */ + oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); + + /* Copy the plan's query string, if available, into the portal */ + query_string = plansource->query_string; + if (query_string) + query_string = pstrdup(query_string); + /* If the plan has parameters, copy them into the portal */ if (plan->nargs > 0) { - oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); /* sizeof(ParamListInfoData) includes the first array element */ paramLI = (ParamListInfo) palloc(sizeof(ParamListInfoData) + (plan->nargs - 1) *sizeof(ParamExternData)); @@ -940,11 +953,12 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan, paramTypByVal, paramTypLen); } } - MemoryContextSwitchTo(oldcontext); } else paramLI = NULL; + MemoryContextSwitchTo(oldcontext); + if (plan->saved) { /* Replan if needed, and increment plan refcount for portal */ @@ -965,7 +979,7 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan, */ PortalDefineQuery(portal, NULL, /* no statement name */ - plansource->query_string, + query_string, plansource->commandTag, stmt_list, cplan); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index a6afecaba60..c3531b58449 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.542.2.1 2008/03/12 23:58:35 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.542.2.2 2008/04/02 18:32:00 tgl Exp $ * * NOTES * this is the "main" module of the postgres backend and @@ -931,6 +931,11 @@ exec_simple_query(const char *query_string) /* Don't display the portal in pg_cursors */ portal->visible = false; + /* + * We don't have to copy anything into the portal, because everything + * we are passsing here is in MessageContext, which will outlive the + * portal anyway. + */ PortalDefineQuery(portal, NULL, query_string, @@ -1354,8 +1359,11 @@ exec_bind_message(StringInfo input_message) CachedPlanSource *psrc; CachedPlan *cplan; Portal portal; + char *query_string; + char *saved_stmt_name; ParamListInfo params; List *plan_list; + MemoryContext oldContext; bool save_log_statement_stats = log_statement_stats; char msec_str[32]; @@ -1459,16 +1467,32 @@ exec_bind_message(StringInfo input_message) else portal = CreatePortal(portal_name, false, false); + /* + * Prepare to copy stuff into the portal's memory context. We do all this + * copying first, because it could possibly fail (out-of-memory) and we + * don't want a failure to occur between RevalidateCachedPlan and + * PortalDefineQuery; that would result in leaking our plancache refcount. + */ + oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); + + /* Copy the plan's query string, if available, into the portal */ + query_string = psrc->query_string; + if (query_string) + query_string = pstrdup(query_string); + + /* Likewise make a copy of the statement name, unless it's unnamed */ + if (stmt_name[0]) + saved_stmt_name = pstrdup(stmt_name); + else + saved_stmt_name = NULL; + /* * Fetch parameters, if any, and store in the portal's memory context. */ if (numParams > 0) { - MemoryContext oldContext; int paramno; - oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); - /* sizeof(ParamListInfoData) includes the first array element */ params = (ParamListInfo) palloc(sizeof(ParamListInfoData) + (numParams - 1) *sizeof(ParamExternData)); @@ -1593,12 +1617,13 @@ exec_bind_message(StringInfo input_message) params->params[paramno].pflags = PARAM_FLAG_CONST; params->params[paramno].ptype = ptype; } - - MemoryContextSwitchTo(oldContext); } else params = NULL; + /* Done storing stuff in portal's context */ + MemoryContextSwitchTo(oldContext); + /* Get the result format codes */ numRFormats = pq_getmsgint(input_message, 2); if (numRFormats > 0) @@ -1625,7 +1650,6 @@ exec_bind_message(StringInfo input_message) } else { - MemoryContext oldContext; List *query_list; /* @@ -1663,8 +1687,8 @@ exec_bind_message(StringInfo input_message) * Define portal and start execution. */ PortalDefineQuery(portal, - stmt_name[0] ? stmt_name : NULL, - psrc->query_string, + saved_stmt_name, + query_string, psrc->commandTag, plan_list, cplan); diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index ca5604a61d5..b977396fb42 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -12,7 +12,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.106 2008/01/01 19:45:55 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.106.2.1 2008/04/02 18:32:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -274,9 +274,7 @@ CreateNewPortal(void) * * Notes: commandTag shall be NULL if and only if the original query string * (before rewriting) was an empty string. Also, the passed commandTag must - * be a pointer to a constant string, since it is not copied. However, - * prepStmtName and sourceText, if provided, are copied into the portal's - * heap context for safekeeping. + * be a pointer to a constant string, since it is not copied. * * If cplan is provided, then it is a cached plan containing the stmts, * and the caller must have done RevalidateCachedPlan(), causing a refcount @@ -285,6 +283,14 @@ CreateNewPortal(void) * If cplan is NULL, then it is the caller's responsibility to ensure that * the passed plan trees have adequate lifetime. Typically this is done by * copying them into the portal's heap context. + * + * The caller is also responsible for ensuring that the passed prepStmtName + * and sourceText (if not NULL) have adequate lifetime. + * + * NB: this function mustn't do much beyond storing the passed values; in + * particular don't do anything that risks elog(ERROR). If that were to + * happen here before storing the cplan reference, we'd leak the plancache + * refcount that the caller is trying to hand off to us. */ void PortalDefineQuery(Portal portal, @@ -299,10 +305,8 @@ PortalDefineQuery(Portal portal, Assert(commandTag != NULL || stmts == NIL); - portal->prepStmtName = prepStmtName ? - MemoryContextStrdup(PortalGetHeapMemory(portal), prepStmtName) : NULL; - portal->sourceText = sourceText ? - MemoryContextStrdup(PortalGetHeapMemory(portal), sourceText) : NULL; + portal->prepStmtName = prepStmtName; + portal->sourceText = sourceText; portal->commandTag = commandTag; portal->stmts = stmts; portal->cplan = cplan;