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

Commit 133924e

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 4ff9c8d commit 133924e

File tree

4 files changed

+36
-20
lines changed
  • sql

    • 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.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
    */< EDBE /span>
    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

    Lines changed: 4 additions & 3 deletions
    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

    Lines changed: 9 additions & 0 deletions
    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

    Lines changed: 6 additions & 0 deletions
    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)
    0