Fix a many-legged critter reported by chifungfan@yahoo.com: under the

tglsfdc · tglsfdc · commit 0147b1934f25 · 2000-08-22T04:06:22.000Z
right circumstances a hash join executed as a DECLARE CURSOR/FETCH
query would crash the backend.  Problem as seen in current sources was
that the hash tables were stored in a context that was a child of
TransactionCommandContext, which got zapped at completion of the FETCH
command --- but cursor cleanup executed at COMMIT expected the tables
to still be valid.  I haven't chased down the details as seen in 7.0.*
but I'm sure it's the same general problem.
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.120 2000/08/06 04:26:40 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.121 2000/08/22 04:06:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -29,6 +29,7 @@
 #include "executor/executor.h"
 #include "libpq/libpq.h"
 #include "miscadmin.h"
+#include "tcop/pquery.h"
 #include "tcop/tcopprot.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
@@ -598,7 +599,7 @@ CopyFrom(Relation rel, bool binary, bool oids, FILE *fp,
 				tuples_read = 0;
 	bool		reading_to_eof = true;
 	RelationInfo *relationInfo;
-	EState	   *estate = makeNode(EState);		/* for ExecConstraints() */
+	EState	   *estate = CreateExecutorState();	/* for ExecConstraints() */
 	TupleTable	tupleTable;
 	TupleTableSlot *slot;
 	Oid			loaded_oid = InvalidOid;
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
@@ -27,7 +27,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.123 2000/08/06 04:26:26 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.124 2000/08/22 04:06:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1574,31 +1574,32 @@ ExecRelCheck(Relation rel, TupleTableSlot *slot, EState *estate)
 {
 	int			ncheck = rel->rd_att->constr->num_check;
 	ConstrCheck *check = rel->rd_att->constr->check;
-	MemoryContext oldContext;
 	ExprContext *econtext;
+	MemoryContext oldContext;
 	List	   *qual;
 	int			i;
 
 	/*
-	 * Make sure econtext, expressions, etc are placed in appropriate context.
+	 * We will use the EState's per-tuple context for evaluating constraint
+	 * expressions.  Create it if it's not already there; if it is, reset it
+	 * to free previously-used storage.
 	 */
-	oldContext = MemoryContextSwitchTo(TransactionCommandContext);
-
-	/*
-	 * Create or reset the exprcontext for evaluating constraint expressions.
-	 */
-	econtext = estate->es_constraint_exprcontext;
+	econtext = estate->es_per_tuple_exprcontext;
 	if (econtext == NULL)
-		estate->es_constraint_exprcontext = econtext =
-			MakeExprContext(NULL, TransactionCommandContext);
+	{
+		oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
+		estate->es_per_tuple_exprcontext = econtext =
+			MakeExprContext(NULL, estate->es_query_cxt);
+		MemoryContextSwitchTo(oldContext);
+	}
 	else
 		ResetExprContext(econtext);
 
 	/*
 	 * If first time through for current result relation, set up econtext's
 	 * range table to refer to result rel, and build expression nodetrees
-	 * for rel's constraint expressions.  All this stuff is kept in
-	 * TransactionCommandContext so it will still be here next time through.
+	 * for rel's constraint expressions.  All this stuff is kept in the
+	 * per-query memory context so it will still be here next time through.
 	 *
 	 * NOTE: if there are multiple result relations (eg, due to inheritance)
 	 * then we leak storage for prior rel's expressions and rangetable.
@@ -1608,7 +1609,14 @@ ExecRelCheck(Relation rel, TupleTableSlot *slot, EState *estate)
 	if (econtext->ecxt_range_table == NIL ||
 		getrelid(1, econtext->ecxt_range_table) != RelationGetRelid(rel))
 	{
-		RangeTblEntry *rte = makeNode(RangeTblEntry);
+		RangeTblEntry *rte;
+
+		/*
+		 * Make sure expressions, etc are placed in appropriate context.
+		 */
+		oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
+
+		rte = makeNode(RangeTblEntry);
 
 		rte->relname = RelationGetRelationName(rel);
 		rte->ref = makeNode(Attr);
@@ -1627,10 +1635,10 @@ ExecRelCheck(Relation rel, TupleTableSlot *slot, EState *estate)
 			qual = (List *) stringToNode(check[i].ccbin);
 			estate->es_result_relation_constraints[i] = qual;
 		}
-	}
 
-	/* Done with building long-lived items */
-	MemoryContextSwitchTo(oldContext);
+		/* Done with building long-lived items */
+		MemoryContextSwitchTo(oldContext);
+	}
 
 	/* Arrange for econtext's scan tuple to be the tuple under test */
 	econtext->ecxt_scantuple = slot;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.64 2000/07/14 22:17:45 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.65 2000/08/22 04:06:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -866,9 +866,8 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
 
 	heapTuple = slot->val;
 
-	/* ----------------
-	 *	get information from the result relation info structure.
-	 * ----------------
+	/*
+	 * Get information from the result relation info structure.
 	 */
 	resultRelationInfo = estate->es_result_relation_info;
 	numIndices = resultRelationInfo->ri_NumIndices;
@@ -877,14 +876,26 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
 	heapRelation = resultRelationInfo->ri_RelationDesc;
 	heapDescriptor = RelationGetDescr(heapRelation);
 
-	/* ----------------
-	 *	Make a temporary expr/memory context for evaluating predicates
-	 *	and functional-index functions.
-	 *	XXX should do this once per command not once per tuple, and
-	 *	just reset it once per tuple.
-	 * ----------------
+	/*
+	 * We will use the EState's per-tuple context for evaluating predicates
+	 * and functional-index functions.  Create it if it's not already there;
+	 * if it is, reset it to free previously-used storage.
 	 */
-	econtext = MakeExprContext(slot, TransactionCommandContext);
+	econtext = estate->es_per_tuple_exprcontext;
+	if (econtext == NULL)
+	{
+		MemoryContext	oldContext;
+
+		oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
+		estate->es_per_tuple_exprcontext = econtext =
+			MakeExprContext(NULL, estate->es_query_cxt);
+		MemoryContextSwitchTo(oldContext);
+	}
+	else
+		ResetExprContext(econtext);
+
+	/* Arrange for econtext's scan tuple to be the tuple under test */
+	econtext->ecxt_scantuple = slot;
 
 	/* ----------------
 	 *	for each index, form and insert the index tuple
@@ -935,8 +946,6 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
 		if (result)
 			pfree(result);
 	}
-
-	FreeExprContext(econtext);
 }
 
 void
diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  *
- *	$Id: nodeHash.c,v 1.50 2000/07/17 03:04:53 tgl Exp $
+ *	$Id: nodeHash.c,v 1.51 2000/08/22 04:06:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -334,7 +334,8 @@ ExecHashTableCreate(Hash *node)
 
 	/* ----------------
 	 *	Initialize the hash table control block.
-	 *	The hashtable control block is just palloc'd from executor memory.
+	 *	The hashtable control block is just palloc'd from the executor's
+	 *	per-query memory context.
 	 * ----------------
 	 */
 	hashtable = (HashJoinTable) palloc(sizeof(HashTableData));
@@ -361,7 +362,7 @@ ExecHashTableCreate(Hash *node)
 	 *	working storage.  See notes in executor/hashjoin.h.
 	 * ----------------
 	 */
-	hashtable->hashCxt = AllocSetContextCreate(TransactionCommandContext,
+	hashtable->hashCxt = AllocSetContextCreate(CurrentMemoryContext,
 											   "HashTableContext",
 											   ALLOCSET_DEFAULT_MINSIZE,
 											   ALLOCSET_DEFAULT_INITSIZE,
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/tcop/pquery.c,v 1.37 2000/07/17 03:05:15 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/tcop/pquery.c,v 1.38 2000/08/22 04:06:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -68,8 +68,8 @@ CreateExecutorState(void)
 	state->es_direction = ForwardScanDirection;
 	state->es_range_table = NIL;
 
-	state->es_into_relation_descriptor = NULL;
 	state->es_result_relation_info = NULL;
+	state->es_into_relation_descriptor = NULL;
 
 	state->es_param_list_info = NULL;
 	state->es_param_exec_vals = NULL;
@@ -78,6 +78,10 @@ CreateExecutorState(void)
 
 	state->es_junkFilter = NULL;
 
+	state->es_query_cxt = CurrentMemoryContext;
+
+	state->es_per_tuple_exprcontext = NULL;
+
 	/* ----------------
 	 *	return the executor state structure
 	 * ----------------
@@ -144,13 +148,11 @@ PreparePortal(char *portalName)
 	}
 
 	/* ----------------
-	 *	 Create the new portal and make its memory context active.
+	 *	 Create the new portal.
 	 * ----------------
 	 */
 	portal = CreatePortal(portalName);
 
-	MemoryContextSwitchTo(PortalGetHeapMemory(portal));
-
 	return portal;
 }
 
@@ -170,8 +172,9 @@ ProcessQuery(Query *parsetree,
 	char	   *tag;
 	bool		isRetrieveIntoPortal;
 	bool		isRetrieveIntoRelation;
-	Portal		portal = NULL;
 	char	   *intoName = NULL;
+	Portal		portal = NULL;
+	MemoryContext oldContext = NULL;
 	QueryDesc  *queryDesc;
 	EState	   *state;
 	TupleDesc	attinfo;
@@ -217,14 +220,18 @@ ProcessQuery(Query *parsetree,
 	if (isRetrieveIntoPortal)
 	{
 		portal = PreparePortal(intoName);
-		/* CurrentMemoryContext is now pointing to portal's context */
+		oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
 		parsetree = copyObject(parsetree);
 		plan = copyObject(plan);
+		/*
+		 * We stay in portal's memory context for now, so that query desc,
+		 * EState, and plan startup info are also allocated in the portal
+		 * context.
+		 */
 	}
 
 	/* ----------------
-	 *	Now we can create the QueryDesc object (this is also in
-	 *	the portal context, if portal retrieve).
+	 *	Now we can create the QueryDesc object.
 	 * ----------------
 	 */
 	queryDesc = CreateQueryDesc(parsetree, plan, dest);
@@ -241,7 +248,7 @@ ProcessQuery(Query *parsetree,
 		queryDesc->dest = (int) None;
 
 	/* ----------------
-	 *	create a default executor state..
+	 *	create a default executor state.
 	 * ----------------
 	 */
 	state = CreateExecutorState();
@@ -279,9 +286,11 @@ ProcessQuery(Query *parsetree,
 					   state,
 					   PortalCleanup);
 
-		MemoryContextSwitchTo(TransactionCommandContext);
+		/* Now we can return to caller's memory context. */
+		MemoryContextSwitchTo(oldContext);
 
 		EndCommand(tag, dest);
+
 		return;
 	}
 
diff --git a/src/include/executor/hashjoin.h b/src/include/executor/hashjoin.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: hashjoin.h,v 1.18 2000/07/12 02:37:30 tgl Exp $
+ * $Id: hashjoin.h,v 1.19 2000/08/22 04:06:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -26,7 +26,7 @@
  * This makes it easy and fast to release the storage when we don't need it
  * anymore.
  *
- * The contexts are made children of TransactionCommandContext, ensuring
+ * The hashtable contexts are made children of the per-query context, ensuring
  * that they will be discarded at end of statement even if the join is
  * aborted early by an error.  (Likewise, any temporary files we make will
  * be cleaned up by the virtual file manager in event of an error.)
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: execnodes.h,v 1.46 2000/08/13 02:50:24 tgl Exp $
+ * $Id: execnodes.h,v 1.47 2000/08/22 04:06:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -223,9 +223,14 @@ typedef struct EState
 	uint32		es_processed;	/* # of tuples processed */
 	Oid			es_lastoid;		/* last oid processed (by INSERT) */
 	List	   *es_rowMark;		/* not good place, but there is no other */
-	/* these two fields are storage space for ExecConstraints(): */
+	MemoryContext es_query_cxt;	/* per-query context in which EState lives */
+	/* this ExprContext is for per-output-tuple operations, such as
+	 * constraint checks and index-value computations.  It can be reset
+	 * for each output tuple.  Note that it will be created only if needed.
+	 */
+	ExprContext *es_per_tuple_exprcontext;
+	/* this field is storage space for ExecConstraints(): */
 	List	  **es_result_relation_constraints;
-	ExprContext *es_constraint_exprcontext;
 	/* Below is to re-evaluate plan qual in READ COMMITTED mode */
 	struct Plan *es_origPlan;
 	Pointer		es_evalPlanQual;

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@`
`7`	`7`	`* Portions Copyright (c) 1996-2000, PostgreSQL, Inc`
`8`	`8`	`* Portions Copyright (c) 1994, Regents of the University of California`
`9`	`9`	`*`
`10`		`- * $Id: hashjoin.h,v 1.18 2000/07/12 02:37:30 tgl Exp $`
	`10`	`+ * $Id: hashjoin.h,v 1.19 2000/08/22 04:06:21 tgl Exp $`
`11`	`11`	`*`
`12`	`12`	`*-------------------------------------------------------------------------`
`13`	`13`	`*/`
`@@ -26,7 +26,7 @@`
`26`	`26`	`* This makes it easy and fast to release the storage when we don't need it`
`27`	`27`	`* anymore.`
`28`	`28`	`*`
`29`		`- * The contexts are made children of TransactionCommandContext, ensuring`
	`29`	`+ * The hashtable contexts are made children of the per-query context, ensuring`
`30`	`30`	`* that they will be discarded at end of statement even if the join is`
`31`	`31`	`* aborted early by an error. (Likewise, any temporary files we make will`
`32`	`32`	`* be cleaned up by the virtual file manager in event of an error.)`