Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 133924e

Browse files
committed
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.
1 parent 4ff9c8d commit 133924e

File tree

4 files changed

+36
-20
lines changed

4 files changed

+36
-20
lines changed

src/backend/executor/nodeSubplan.c

+17-17
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.102 2010/07/12 17:01:05 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.103 2010/07/28 04:50:50 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -89,7 +89,6 @@ ExecHashSubPlan(SubPlanState *node,
8989
{
9090
SubPlan *subplan = (SubPlan *) node->xprstate.expr;
9191
PlanState *planstate = node->planstate;
92-
ExprContext *innerecontext = node->innerecontext;
9392
TupleTableSlot *slot;
9493

9594
/* Shouldn't have any direct correlation Vars */
@@ -126,12 +125,6 @@ ExecHashSubPlan(SubPlanState *node,
126125
* it still needs to free the tuple.
127126
*/
128127

129-
/*
130-
* Since the hashtable routines will use innerecontext's per-tuple memory
131-
* as working memory, be sure to reset it for each tuple.
132-
*/
133-
ResetExprContext(innerecontext);
134-
135128
/*
136129
* If the LHS is all non-null, probe for an exact match in the main hash
137130
* table. If we find one, the result is TRUE. Otherwise, scan the
@@ -438,7 +431,6 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
438431
PlanState *planstate = node->planstate;
439432
int ncols = list_length(subplan->paramIds);
440433
ExprContext *innerecontext = node->innerecontext;
441-
MemoryContext tempcxt = innerecontext->ecxt_per_tuple_memory;
442434
MemoryContext oldcontext;
443435
int nbuckets;
444436
TupleTableSlot *slot;
@@ -460,7 +452,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
460452
* If it's not necessary to distinguish FALSE and UNKNOWN, then we don't
461453
* need to store subplan output rows that contain NULL.
462454
*/
463-
MemoryContextReset(node->tablecxt);
455+
MemoryContextReset(node->hashtablecxt);
464456
node->hashtable = NULL;
465457
node->hashnulls = NULL;
466458
node->havehashrows = false;
@@ -476,8 +468,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
476468
node->tab_hash_funcs,
477469
nbuckets,
478470
sizeof(TupleHashEntryData),
479-
node->tablecxt,
480-
tempcxt);
471+
node->hashtablecxt,
472+
node->hashtempcxt);
481473

482474
if (!subplan->unknownEqFalse)
483475
{
@@ -495,8 +487,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
495487
node->tab_hash_funcs,
496488
nbuckets,
497489
sizeof(TupleHashEntryData),
498-
node->tablecxt,
499-
tempcxt);
490+
node->hashtablecxt,
491+
node->hashtempcxt);
500492
}
501493

502494
/*
@@ -555,7 +547,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
555547

556548
/*
557549
* Reset innerecontext after each inner tuple to free any memory used
558-
* in hash computation or comparison routines.
550+
* during ExecProject.
559551
*/
560552
ResetExprContext(innerecontext);
561553
}
@@ -680,7 +672,8 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
680672
sstate->projRight = NULL;
681673
sstate->hashtable = NULL;
682674
sstate->hashnulls = NULL;
683-
sstate->tablecxt = NULL;
675+
sstate->hashtablecxt = NULL;
676+
sstate->hashtempcxt = NULL;
684677
sstate->innerecontext = NULL;
685678
sstate->keyColIdx = NULL;
686679
sstate->tab_hash_funcs = NULL;
@@ -730,12 +723,19 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
730723
ListCell *l;
731724

732725
/* We need a memory context to hold the hash table(s) */
733-
sstate->tablecxt =
726+
sstate->hashtablecxt =
734727
AllocSetContextCreate(CurrentMemoryContext,
735728
"Subplan HashTable Context",
736729
ALLOCSET_DEFAULT_MINSIZE,
737730
ALLOCSET_DEFAULT_INITSIZE,
738731
ALLOCSET_DEFAULT_MAXSIZE);
732+
/* and a small one for the hash tables to use as temp storage */
733+
sstate->hashtempcxt =
734+
AllocSetContextCreate(CurrentMemoryContext,
735+
"Subplan HashTable Temp Context",
736+
ALLOCSET_SMALL_MINSIZE,
737+
ALLOCSET_SMALL_INITSIZE,
738+
ALLOCSET_SMALL_MAXSIZE);
739739
/* and a short-lived exprcontext for function evaluation */
740740
sstate->innerecontext = CreateExprContext(estate);
741741
/* Silly little array of column numbers 1..n */

src/include/nodes/execnodes.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.219 2010/02/26 02:01:25 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.220 2010/07/28 04:50:50 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -699,8 +699,9 @@ typedef struct SubPlanState
699699
TupleHashTable hashnulls; /* hash table for rows with null(s) */
700700
bool havehashrows; /* TRUE if hashtable is not empty */
701701
bool havenullrows; /* TRUE if hashnulls is not empty */
702-
MemoryContext tablecxt; /* memory context containing tables */
703-
ExprContext *innerecontext; /* working context for comparisons */
702+
MemoryContext hashtablecxt; /* memory context containing hash tables */
703+
MemoryContext hashtempcxt; /* temp memory context for hash tables */
704+
ExprContext *innerecontext; /* econtext for computing inner tuples */
704705
AttrNumber *keyColIdx; /* control data for hash tables */
705706
FmgrInfo *tab_hash_funcs; /* hash functions for table datatype(s) */
706707
FmgrInfo *tab_eq_funcs; /* equality functions for table datatype(s) */

src/test/regress/expected/subselect.out

+9
Original file line numberDiff line numberDiff line change
@@ -521,3 +521,12 @@ from
521521
-----
522522
(0 rows)
523523

524+
--
525+
-- Test case for premature memory release during hashing of subplan output
526+
--
527+
select '1'::text in (select '1'::name union all select '1'::name);
528+
?column?
529+
----------
530+
t
531+
(1 row)
532+

src/test/regress/sql/subselect.sql

+6
Original file line numberDiff line numberDiff line change
@@ -335,3 +335,9 @@ from
335335
from int8_tbl) sq0
336336
join
337337
int4_tbl i4 on dummy = i4.f1;
338+
339+
--
340+
-- Test case for premature memory release during hashing of subplan output
341+
--
342+
343+
select '1'::text in (select '1'::name union all select '1'::name);

0 commit comments

Comments
 (0)