Revert my bad decision of about a year ago to make PortalDefineQuery

responsible for copying the query string into the new Portal.  Such copying
is unnecessary in the common code path through exec_simple_query, and in
this case it can be enormously expensive because the string might contain
a large number of individual commands; we were copying the entire, long
string for each command, resulting in O(N^2) behavior for N commands.
(This is the cause of bug #4079.)  A second problem with it is that
PortalDefineQuery really can't risk error, because if it elog's before
having set up the Portal, we will leak the plancache refcount that the
caller is trying to hand off to the portal.  So go back to the design in
which the caller is responsible for making sure everything is copied into
the portal if necessary.

1 files changed, 33 insertions, 9 deletions

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 95bea2ef8a9..d44e69fb10a 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.547 2008/03/26 18:48:59 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.548 2008/04/02 18:31:50 tgl Exp $
  *
  * NOTES
  *	  this is the "main" module of the postgres backend and
@@ -933,6 +933,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,
@@ -1356,8 +1361,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];
 
@@ -1462,15 +1470,31 @@ exec_bind_message(StringInfo input_message)
 		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));
@@ -1595,12 +1619,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)
@@ -1627,7 +1652,6 @@ exec_bind_message(StringInfo input_message)
 	}
 	else
 	{
-		MemoryContext oldContext;
 		List	   *query_list;
 
 		/*
@@ -1665,8 +1689,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);