8000 Fix potential failure when hashing the output of a subplan that produces · percona/postgres@7c294bf · GitHub
[go: up one dir, main page]
Skip to content

Commit 7c294bf

Browse files
tglsfdctglsfdc
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 003c598 commit 7c294bf

File tree

4 files changed

+36
-20
lines changed

4 files changed

+36
-20
lines changed

‎src/backend/executor/nodeSubplan.c

Lines changed: 17 additions & 17 deletions
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.70.2.3 2007/04/26 23:25:08 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.70.2.4 2010/07/28 04:51:27 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -78,7 +78,6 @@ ExecHashSubPlan(SubPlanState *node,
7878
{
7979
SubPlan *subplan = (SubPlan *) node->xprstate.expr;
8080
PlanState *planstate = node->planstate;
81-
ExprContext *innerecontext = node->innerecontext;
8281
TupleTableSlot *slot;
8382

8483
/* Shouldn't have any direct correlation Vars */
@@ -115,12 +114,6 @@ ExecHashSubPlan(SubPlanState *node,
115114
* it still needs to free the tuple.
116115
*/
117116

118-
/*
119-
* Since the hashtable routines will use innerecontext's per-tuple memory
120-
* as working memory, be sure to reset it for each tuple.
121-
*/
122-
ResetExprContext(innerecontext);
123-
124117
/*
125118
* If the LHS is all non-null, probe for an exact match in the main hash
126119
* table. If we find one, the result is TRUE. Otherwise, scan the
@@ -465,7 +458,6 @@ buildSubPlanHash(SubPlanState *node)
465458
PlanState *planstate = node->planstate;
466459
int ncols = list_length(node->exprs);
467460
ExprContext *innerecontext = node->innerecontext;
468-
MemoryContext tempcxt = innerecontext->ecxt_per_tuple_memory;
469461
MemoryContext oldcontext;
470462
int nbuckets;
471463
TupleTableSlot *slot;
@@ -488,7 +480,7 @@ buildSubPlanHash(SubPlanState *node)
488480
* If it's not necessary to distinguish FALSE and UNKNOWN, then we don't
489481
* need to store subplan output rows that contain NULL.
490482
*/
491-
MemoryContextReset(node->tablecxt);
483+
MemoryContextReset(node->hashtablecxt);
492484
node->hashtable = NULL;
493485
node->hashnulls = NULL;
494486
node->havehashrows = false;
@@ -504,8 +496,8 @@ buildSubPlanHash(SubPlanState *node)
504496
node->hashfunctions,
505497
nbuckets,
506498
sizeof(TupleHashEntryData),
507-
node->tablecxt,
508-
tempcxt);
499+
node->hashtablecxt,
500+
node->hashtempcxt);
509501

510502
if (!subplan->unknownEqFalse)
511503
{
@@ -523,8 +515,8 @@ buildSubPlanHash(SubPlanState *node)
523515
node->hashfunctions,
524516
nbuckets,
525517
sizeof(TupleHashEntryData),
526-
node->tablecxt,
527-
tempcxt);
518+
node->hashtablecxt,
519+
node->hashtempcxt);
528520
}
529521

530522
/*
@@ -583,7 +575,7 @@ buildSubPlanHash(SubPlanState *node)
583575

584576
/*
585577
* Reset innerecontext after each inner tuple to free any memory used
586-
* in hash computation or comparison routines.
578+
* during ExecProject.
587579
*/
588580
ResetExprContext(innerecontext);
589581
}
@@ -699,7 +691,8 @@ ExecInitSubPlan(SubPlanState *node, EState *estate)
699691
node->projRight = NULL;
700692
node->hashtable = NULL;
701693
node->hashnulls = NULL;
702-
node->tablecxt = NULL;
694+
node->hashtablecxt = NULL;
695+
node->hashtempcxt = NULL;
703696
node->innerecontext = NULL;
704697
node->keyColIdx = NULL;
705698
node->eqfunctions = NULL;
@@ -775,12 +768,19 @@ ExecInitSubPlan(SubPlanState *node, EState *estate)
775768
ListCell *lexpr;
776769

777770
/* We need a memory context to hold the hash table(s) */
778-
node->tablecxt =
771+
node->hashtablecxt =
779772
AllocSetContextCreate(CurrentMemoryContext,
780773
"Subplan HashTable Context",
781774
ALLOCSET_DEFAULT_MINSIZE,
782775
ALLOCSET_DEFAULT_INITSIZE,
783776
ALLOCSET_DEFAULT_MAXSIZE);
777+
/* and a small one for the hash tables to use as temp storage */
778+
node->hashtempcxt =
779+
AllocSetContextCreate(CurrentMemoryContext,
780+
"Subplan HashTable Temp Context",
781+
ALLOCSET_SMALL_MINSIZE,
782+
ALLOCSET_SMALL_INITSIZE,
783+
ALLOCSET_SMALL_MAXSIZE);
784784
/* and a short-lived exprcontext for function evaluation */
785785
node->innerecontext = CreateExprContext(estate);
786786
/* Silly little array of column numbers 1..n */

‎src/include/nodes/execnodes.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2005, 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.139.2.4 2007/04/26 23:25:09 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.139.2.5 2010/07/28 04:51:27 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -581,8 +581,9 @@ typedef struct SubPlanState
581581
TupleHashTable hashnulls; /* hash table for rows with null(s) */
582582
bool havehashrows; /* TRUE if hashtable is not empty */
583583
bool havenullrows; /* TRUE if hashnulls is not empty */
584-
MemoryContext tablecxt; /* memory context containing tables */
585-
ExprContext *innerecontext; /* working context for comparisons */
584+
MemoryContext hashtablecxt; /* memory context containing hash tables */
585+
MemoryContext hashtempcxt; /* temp memory context for hash tables */
586+
ExprContext *innerecontext; /* econtext for computing inner tuples */
586587
AttrNumber *keyColIdx; /* control data for hash tables */
587588
FmgrInfo *eqfunctions; /* comparison functions for hash tables */
588589
FmgrInfo *hashfunctions; /* lookup data for hash functions */

‎src/test/regress/expected/subselect.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,3 +449,12 @@ from
449449
-----
450450
(0 rows)
451451

452+
--
453+
-- Test case for premature memory release during hashing of subplan output
454+
--
455+
select '1'::text in (select '1'::name union all select '1'::name);
456+
?column?
457+
----------
458+
t
459+
(1 row)
460+

‎src/test/regress/sql/subselect.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,9 @@ from
289289
from int8_tbl) sq0
290290
join
291291
int4_tbl i4 on dummy = i4.f1;
292+
293+
--
294+
-- Test case for premature memory release during hashing of subplan output
295+
--
296+
297+
select '1'::text in (select '1'::name union all select '1'::name);

0 commit comments

Comments
 (0)
0