From 0147b1934f251183d3614bca011bf21205890835 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 22 Aug 2000 04:06:22 +0000 Subject: [PATCH] Fix a many-legged critter reported by chifungfan@yahoo.com: under the 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. --- src/backend/commands/copy.c | 5 ++-- src/backend/executor/execMain.c | 42 +++++++++++++++++++------------- src/backend/executor/execUtils.c | 35 ++++++++++++++++---------- src/backend/executor/nodeHash.c | 7 +++--- src/backend/tcop/pquery.c | 31 ++++++++++++++--------- src/include/executor/hashjoin.h | 4 +-- src/include/nodes/execnodes.h | 11 ++++++--- 7 files changed, 84 insertions(+), 51 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index c71d081809..4b81a35c12 100644 --- 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 index e66107ce7d..2db826144d 100644 --- 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 index 39e3d5cd48..63c1e9e157 100644 --- 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 index f63ffe4943..682afdba4a 100644 --- 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 index 104a82cde3..172f6fe467 100644 --- 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 index 8d4cb98469..25a8174f29 100644 --- 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 index 6e691c1d78..1ec14f4a96 100644 --- 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; -- GitLab