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.

diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index fa8d63513e45a5ac79ff5b3228bb634bcf2a5d63..a8fb87a18ce8d166660dabbba188c6d586ea96b6 100644 (file)
--- 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.92 2008/01/01 19:45:49 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.92.2.1 2010/07/28 04:51:14 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -78,7 +78,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 */
@@ -115,12 +114,6 @@ ExecHashSubPlan(SubPlanState *node,
         * it still needs to free the tuple.
         */
 
-       /*
-        * 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
@@ -427,7 +420,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;
@@ -449,7 +441,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;
@@ -465,8 +457,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
                                                                                  node->tab_hash_funcs,
                                                                                  nbuckets,
                                                                                  sizeof(TupleHashEntryData),
-                                                                                 node->tablecxt,
-                                                                                 tempcxt);
+                                                                                 node->hashtablecxt,
+                                                                                 node->hashtempcxt);
 
        if (!subplan->unknownEqFalse)
        {
@@ -484,8 +476,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
                                                                                          node->tab_hash_funcs,
                                                                                          nbuckets,
                                                                                          sizeof(TupleHashEntryData),
-                                                                                         node->tablecxt,
-                                                                                         tempcxt);
+                                                                                         node->hashtablecxt,
+                                                                                         node->hashtempcxt);
        }
 
        /*
@@ -544,7 +536,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);
        }
@@ -669,7 +661,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;
@@ -717,12 +710,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 e977b9f8705dd7955f5a82a0aff0e9bb795ec979..1ff2d0a00023c65138e5f95ac684b7fcffce7840 100644 (file)
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.183 2008/01/01 19:45:58 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.183.2.1 2010/07/28 04:51:14 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -618,8 +618,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 a3be2997b505eef325b90925b405b112fe8d2c1a..468829a572c665f2fa56fc1e9d5f73b154e49ccd 100644 (file)
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -492,3 +492,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 bfc0c33f2455c8c3cd7a28b284848a2baf827e0c..6e1eda157a433acd0d6e8a5a14ba018b2f1a82e2 100644 (file)
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -321,3 +321,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);