aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2010-07-28 04:51:08 +0000
committerTom Lane <tgl@sss.pgh.pa.us>2010-07-28 04:51:08 +0000
commit5a188dcb87d73c2ab8496186b412c1bd99ca05cb (patch)
tree11e6cfcece9dc1600f3d59ff1f0a6a459df13871
parent29789a8b7a43e3ab525e9dff1b2d800f22c51c4e (diff)
downloadpostgresql-5a188dcb87d73c2ab8496186b412c1bd99ca05cb.tar.gz
postgresql-5a188dcb87d73c2ab8496186b412c1bd99ca05cb.zip
Fix potential failure when hashing the output of a subplan that produces
a pass-by-reference datatype with a nontrivial projection step. We were using the same memory context for the projection operation as for the temporary context used by the hashtable routines in execGrouping.c. However, the hashtable routines feel free to reset their temp context at any time, which'd lead to destroying input data that was still needed. Report and diagnosis by Tao Ma. Back-patch to 8.1, where the problem was introduced by the changes that allowed us to work with "virtual" tuples instead of materializing intermediate tuple values everywhere. The earlier code looks quite similar, but it doesn't suffer the problem because the data gets copied into another context as a result of having to materialize ExecProject's output tuple.
-rw-r--r--src/backend/executor/nodeSubplan.c34
-rw-r--r--src/include/nodes/execnodes.h7
-rw-r--r--src/test/regress/expected/subselect.out9
-rw-r--r--src/test/regress/sql/subselect.sql6
4 files changed, 36 insertions, 20 deletions
diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index 016dbc378b4..d03416a56b4 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.99 2009/06/11 14:48:57 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.99.2.1 2010/07/28 04:51:08 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -89,7 +89,6 @@ ExecHashSubPlan(SubPlanState *node,
{
SubPlan *subplan = (SubPlan *) node->xprstate.expr;
PlanState *planstate = node->planstate;
- ExprContext *innerecontext = node->innerecontext;
TupleTableSlot *slot;
/* Shouldn't have any direct correlation Vars */
@@ -127,12 +126,6 @@ ExecHashSubPlan(SubPlanState *node,
*/
/*
- * Since the hashtable routines will use innerecontext's per-tuple memory
- * as working memory, be sure to reset it for each tuple.
- */
- ResetExprContext(innerecontext);
-
- /*
* If the LHS is all non-null, probe for an exact match in the main hash
* table. If we find one, the result is TRUE. Otherwise, scan the
* partly-null table to see if there are any rows that aren't provably
@@ -438,7 +431,6 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
PlanState *planstate = node->planstate;
int ncols = list_length(subplan->paramIds);
ExprContext *innerecontext = node->innerecontext;
- MemoryContext tempcxt = innerecontext->ecxt_per_tuple_memory;
MemoryContext oldcontext;
int nbuckets;
TupleTableSlot *slot;
@@ -460,7 +452,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
* If it's not necessary to distinguish FALSE and UNKNOWN, then we don't
* need to store subplan output rows that contain NULL.
*/
- MemoryContextReset(node->tablecxt);
+ MemoryContextReset(node->hashtablecxt);
node->hashtable = NULL;
node->hashnulls = NULL;
node->havehashrows = false;
@@ -476,8 +468,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
node->tab_hash_funcs,
nbuckets,
sizeof(TupleHashEntryData),
- node->tablecxt,
- tempcxt);
+ node->hashtablecxt,
+ node->hashtempcxt);
if (!subplan->unknownEqFalse)
{
@@ -495,8 +487,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
node->tab_hash_funcs,
nbuckets,
sizeof(TupleHashEntryData),
- node->tablecxt,
- tempcxt);
+ node->hashtablecxt,
+ node->hashtempcxt);
}
/*
@@ -555,7 +547,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
/*
* Reset innerecontext after each inner tuple to free any memory used
- * in hash computation or comparison routines.
+ * during ExecProject.
*/
ResetExprContext(innerecontext);
}
@@ -680,7 +672,8 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
sstate->projRight = NULL;
sstate->hashtable = NULL;
sstate->hashnulls = NULL;
- sstate->tablecxt = NULL;
+ sstate->hashtablecxt = NULL;
+ sstate->hashtempcxt = NULL;
sstate->innerecontext = NULL;
sstate->keyColIdx = NULL;
sstate->tab_hash_funcs = NULL;
@@ -731,12 +724,19 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
ListCell *l;
/* We need a memory context to hold the hash table(s) */
- sstate->tablecxt =
+ sstate->hashtablecxt =
AllocSetContextCreate(CurrentMemoryContext,
"Subplan HashTable Context",
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
+ /* and a small one for the hash tables to use as temp storage */
+ sstate->hashtempcxt =
+ AllocSetContextCreate(CurrentMemoryContext,
+ "Subplan HashTable Temp Context",
+ ALLOCSET_SMALL_MINSIZE,
+ ALLOCSET_SMALL_INITSIZE,
+ ALLOCSET_SMALL_MAXSIZE);
/* and a short-lived exprcontext for function evaluation */
sstate->innerecontext = CreateExprContext(estate);
/* Silly little array of column numbers 1..n */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index c2e1bd8c470..23a86e90848 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.205.2.2 2010/01/05 23:25:44 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.205.2.3 2010/07/28 04:51:08 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -670,8 +670,9 @@ typedef struct SubPlanState
TupleHashTable hashnulls; /* hash table for rows with null(s) */
bool havehashrows; /* TRUE if hashtable is not empty */
bool havenullrows; /* TRUE if hashnulls is not empty */
- MemoryContext tablecxt; /* memory context containing tables */
- ExprContext *innerecontext; /* working context for comparisons */
+ MemoryContext hashtablecxt; /* memory context containing hash tables */
+ MemoryContext hashtempcxt; /* temp memory context for hash tables */
+ ExprContext *innerecontext; /* econtext for computing inner tuples */
AttrNumber *keyColIdx; /* control data for hash tables */
FmgrInfo *tab_hash_funcs; /* hash functions for table datatype(s) */
FmgrInfo *tab_eq_funcs; /* equality functions for table datatype(s) */
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index e839ad7f964..7f8c05bc800 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -521,3 +521,12 @@ from
-----
(0 rows)
+--
+-- Test case for premature memory release during hashing of subplan output
+--
+select '1'::text in (select '1'::name union all select '1'::name);
+ ?column?
+----------
+ t
+(1 row)
+
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index f6657fa535c..9d13c39c8ab 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -335,3 +335,9 @@ from
from int8_tbl) sq0
join
int4_tbl i4 on dummy = i4.f1;
+
+--
+-- Test case for premature memory release during hashing of subplan output
+--
+
+select '1'::text in (select '1'::name union all select '1'::name);