Fix potential failure when hashing the output of a subplan that produces
authorTom Lane <[email protected]>
Wed, 28 Jul 2010 04:51:21 +0000 (04:51 +0000)
committerTom Lane <[email protected]>
Wed, 28 Jul 2010 04:51:21 +0000 (04:51 +0000)
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- 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.

src/backend/executor/nodeSubplan.c
src/include/nodes/execnodes.h
src/test/regress/expected/subselect.out
src/test/regress/sql/subselect.sql

index 02a00b15a7b4a3db048c0595db2402208a12622a..e456fdb619844f58bc286997c867a4e47c4ce685 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.80.2.3 2007/04/26 23:24:56 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.80.2.4 2010/07/28 04:51:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -79,7 +79,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 */
@@ -116,12 +115,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
@@ -419,7 +412,6 @@ buildSubPlanHash(SubPlanState *node)
    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;
@@ -441,7 +433,7 @@ buildSubPlanHash(SubPlanState *node)
     * 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;
@@ -457,8 +449,8 @@ buildSubPlanHash(SubPlanState *node)
                                          node->hashfunctions,
                                          nbuckets,
                                          sizeof(TupleHashEntryData),
-                                         node->tablecxt,
-                                         tempcxt);
+                                         node->hashtablecxt,
+                                         node->hashtempcxt);
 
    if (!subplan->unknownEqFalse)
    {
@@ -476,8 +468,8 @@ buildSubPlanHash(SubPlanState *node)
                                              node->hashfunctions,
                                              nbuckets,
                                              sizeof(TupleHashEntryData),
-                                             node->tablecxt,
-                                             tempcxt);
+                                             node->hashtablecxt,
+                                             node->hashtempcxt);
    }
 
    /*
@@ -536,7 +528,7 @@ buildSubPlanHash(SubPlanState *node)
 
        /*
         * Reset innerecontext after each inner tuple to free any memory used
-        * in hash computation or comparison routines.
+        * during ExecProject.
         */
        ResetExprContext(innerecontext);
    }
@@ -654,7 +646,8 @@ ExecInitSubPlan(SubPlanState *node, EState *estate, int eflags)
    node->projRight = NULL;
    node->hashtable = NULL;
    node->hashnulls = NULL;
-   node->tablecxt = NULL;
+   node->hashtablecxt = NULL;
+   node->hashtempcxt = NULL;
    node->innerecontext = NULL;
    node->keyColIdx = NULL;
    node->eqfunctions = NULL;
@@ -735,12 +728,19 @@ ExecInitSubPlan(SubPlanState *node, EState *estate, int eflags)
        ListCell   *l;
 
        /* We need a memory context to hold the hash table(s) */
-       node->tablecxt =
+       node->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 */
+       node->hashtempcxt =
+           AllocSetContextCreate(CurrentMemoryContext,
+                                 "Subplan HashTable Temp Context",
+                                 ALLOCSET_SMALL_MINSIZE,
+                                 ALLOCSET_SMALL_INITSIZE,
+                                 ALLOCSET_SMALL_MAXSIZE);
        /* and a short-lived exprcontext for function evaluation */
        node->innerecontext = CreateExprContext(estate);
        /* Silly little array of column numbers 1..n */
index e981a6a933bc7fb11af9c3033418141d0d98cd8f..84bf53f956e9766e22a5cc0207c73111a958b2da 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.161.2.2 2007/04/26 23:24:57 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.161.2.3 2010/07/28 04:51:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -589,8 +589,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   *eqfunctions;    /* comparison functions for hash tables */
    FmgrInfo   *hashfunctions;  /* lookup data for hash functions */
index 339b46f29729d5d6cb65ef982f04d38e5d7f5871..7dd2db728bd2b65ae8dc4385293ff746a1cb1ade 100644 (file)
@@ -449,3 +449,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)
+
index 46b46b4c45e160065ceafed0a687bcd9d8a65447..257ff9a6a598f5af06dbd9335b010b3119985745 100644 (file)
@@ -289,3 +289,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);