Arrange for ValuesScan to keep per-sublist expression eval state in a

temporary context that can be reset when advancing to the next sublist.
This is faster and more thorough at recovering space than the previous
method; moreover it will do the right thing if something in the sublist
tries to register an expression context callback.

src/backend/executor/nodeValuesscan.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeValuesscan.c,v 1.1 2006/08/02 01:59:45 joe Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeValuesscan.c,v 1.2 2006/08/02 18:58:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -30,9 +30,6 @@
 
 
 static TupleTableSlot *ValuesNext(ValuesScanState *node);
-static void ExecMakeValuesResult(List *targetlist,
-					 ExprContext *econtext,
-					 TupleTableSlot *slot);
 
 
 /* ----------------------------------------------------------------
@@ -61,7 +58,7 @@ ValuesNext(ValuesScanState *node)
 	estate = node->ss.ps.state;
 	direction = estate->es_direction;
 	slot = node->ss.ss_ScanTupleSlot;
-	econtext = node->ss.ps.ps_ExprContext;
+	econtext = node->rowcontext;
 
 	/*
 	 * Get the next tuple. Return NULL if no more tuples.
@@ -85,57 +82,58 @@ ValuesNext(ValuesScanState *node)
 			exprlist = NIL;
 	}
 
-	if (exprlist)
-	{
-		List		   *init_exprlist;
-
-		init_exprlist = (List *) ExecInitExpr((Expr *) exprlist,
-											  (PlanState *) node);
-		ExecMakeValuesResult(init_exprlist,
-							 econtext,
-							 slot);
-		list_free_deep(init_exprlist);
-	}
-	else
+	/*
+	 * Always clear the result slot; this is appropriate if we are at the
+	 * end of the data, and if we're not, we still need it as the first step
+	 * of the store-virtual-tuple protocol.  It seems wise to clear the slot
+	 * before we reset the context it might have pointers into.
+	 */
 	ExecClearTuple(slot);
 
-	return slot;
-}
-
-/*
- *		ExecMakeValuesResult
- *
- * Evaluate a values list, store into a virtual slot.
- */
-static void
-ExecMakeValuesResult(List *targetlist,
-					 ExprContext *econtext,
-					 TupleTableSlot *slot)
-{
+	if (exprlist)
+	{
 		MemoryContext oldContext;
+		List	   *exprstatelist;
 		Datum	   *values;
 		bool	   *isnull;
 		ListCell   *lc;
-	int					resind = 0;
-
-	/* caller should have checked all targetlists are the same length */
-	Assert(list_length(targetlist) == slot->tts_tupleDescriptor->natts);
+		int			resind;
 
 		/*
-	 * Prepare to build a virtual result tuple.
+		 * Get rid of any prior cycle's leftovers.  We use ReScanExprContext
+		 * not just ResetExprContext because we want any registered shutdown
+		 * callbacks to be called.
 		 */
-	ExecClearTuple(slot);
-	values = slot->tts_values;
-	isnull = slot->tts_isnull;
+		ReScanExprContext(econtext);
 
 		/*
-	 * Switch to short-lived context for evaluating the row.
-	 * Reset per-tuple memory context before each row.
+		 * Build the expression eval state in the econtext's per-tuple
+		 * memory.  This is a tad unusual, but we want to delete the eval
+		 * state again when we move to the next row, to avoid growth of
+		 * memory requirements over a long values list.
 		 */
-	ResetExprContext(econtext);
 		oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
 
-	foreach(lc, targetlist)
+		/*
+		 * Pass NULL, not my plan node, because we don't want anything
+		 * in this transient state linking into permanent state.  The
+		 * only possibility is a SubPlan, and there shouldn't be any
+		 * (any subselects in the VALUES list should be InitPlans).
+		 */
+		exprstatelist = (List *) ExecInitExpr((Expr *) exprlist, NULL);
+
+		/* parser should have checked all sublists are the same length */
+		Assert(list_length(exprstatelist) == slot->tts_tupleDescriptor->natts);
+
+		/*
+		 * Compute the expressions and build a virtual result tuple.
+		 * We already did ExecClearTuple(slot).
+		 */
+		values = slot->tts_values;
+		isnull = slot->tts_isnull;
+
+		resind = 0;
+		foreach(lc, exprstatelist)
 		{
 			ExprState *estate = (ExprState *) lfirst(lc);
 
@@ -152,6 +150,9 @@ ExecMakeValuesResult(List *targetlist,
 		 * And return the virtual tuple.
 		 */
 		ExecStoreVirtualTuple(slot);
+	}
+
+	return slot;
 }
 
 
@@ -186,7 +187,6 @@ ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags)
 	ListCell		   *vtl;
 	int					i;
 	PlanState		   *planstate;
-	ExprContext		   *econtext;
 
 	/*
 	 * ValuesScan should not have any children.
@@ -203,12 +203,17 @@ ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags)
 
 	/*
 	 * Miscellaneous initialization
-	 *
-	 * create expression context for node
 	 */
 	planstate = &scanstate->ss.ps;
+
+	/*
+	 * Create expression contexts.	We need two, one for per-sublist
+	 * processing and one for execScan.c to use for quals and projections.
+	 * We cheat a little by using ExecAssignExprContext() to build both.
+	 */
+	ExecAssignExprContext(estate, planstate);
+	scanstate->rowcontext = planstate->ps_ExprContext;
 	ExecAssignExprContext(estate, planstate);
-	econtext = planstate->ps_ExprContext;
 
 #define VALUESSCAN_NSLOTS 2
 
@@ -282,9 +287,11 @@ void
 ExecEndValuesScan(ValuesScanState *node)
 {
 	/*
-	 * Free the exprcontext
+	 * Free both exprcontexts
 	 */
 	ExecFreeExprContext(&node->ss.ps);
+	node->ss.ps.ps_ExprContext = node->rowcontext;
+	ExecFreeExprContext(&node->ss.ps);
 
 	/*
 	 * clean out the tuple table

src/include/nodes/execnodes.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.156 2006/08/02 01:59:47 joe Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.157 2006/08/02 18:58:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1044,18 +1044,25 @@ typedef struct FunctionScanState
 /* ----------------
  *	 ValuesScanState information
  *
- *		Values nodes are used to scan the results of a
- *		values list appearing in FROM or INSERT
+ *		ValuesScan nodes are used to scan the results of a VALUES list
  *
+ *		rowcontext			per-expression-list context
  *		exprlists			array of expression lists being evaluated
  *		array_len			size of array
  *		curr_idx			current array index (0-based)
  *		marked_idx			marked position (for mark/restore)
+ *
+ *	Note: ss.ps.ps_ExprContext is used to evaluate any qual or projection
+ *	expressions attached to the node.  We create a second ExprContext,
+ *	rowcontext, in which to build the executor expression state for each
+ *	Values sublist.  Resetting this context lets us get rid of expression
+ *	state for each row, avoiding major memory leakage over a long values list.
  * ----------------
  */
 typedef struct ValuesScanState
 {
 	ScanState	ss;				/* its first field is NodeTag */
+	ExprContext *rowcontext;
 	List	  **exprlists;
 	int			array_len;
 	int			curr_idx;