8000 Repair failure with SubPlans in multi-row VALUES lists. · postgrespro/postgres@9b63c13 · GitHub
[go: up one dir, main page]
Skip to content

    • Commit 9b63c13

    Browse files
    tglsfdctglsfdc
    committed
    Repair failure with SubPlans in multi-row VALUES lists.
    When nodeValuesscan.c was written, it was impossible to have a SubPlan in VALUES --- any sub-SELECT there would have to be uncorrelated and thereby would produce an InitPlan instead. We therefore took a shortcut in the logic that throws away a ValuesScan's per-row expression evaluation data structures. This was broken by the introduction of LATERAL however; a sub-SELECT containing a lateral reference produces a correlated SubPlan. The cleanest fix for this would be to give up the optimization of discarding the expression eval state. But that still seems pretty unappetizing for long VALUES lists. It seems to work to just prevent the subexpressions from hooking into the ValuesScan node's subPlan list, so let's do that and see how well it works. (If this breaks, due to additional connections between the subexpressions and the outer query structures, we might consider compromises like throwing away data only for VALUES rows not containing SubPlans.) Per bug #14924 from Christian Duta. Back-patch to 9.3 where LATERAL was introduced. Discussion: https://postgr.es/m/20171124120836.1463.5310@wrigleys.postgresql.org
    1 parent ab97aaa commit 9b63c13

    File tree

    3 files changed

    +56
    -5
    lines changed

    3 files changed

    +56
    -5
    lines changed

    ‎src/backend/executor/nodeValuesscan.c

    Lines changed: 16 additions & 5 deletions
    Original file line numberDiff line numberDiff line change
    @@ -92,6 +92,7 @@ ValuesNext(ValuesScanState *node)
    9292
    if (exprlist)
    9393
    {
    9494
    MemoryContext oldContext;
    95+
    List *oldsubplans;
    9596
    List *exprstatelist;
    9697
    Datum *values;
    9798
    bool *isnull;
    @@ -114,12 +115,22 @@ ValuesNext(ValuesScanState *node)
    114115
    oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
    115116

    116117
    /*
    117-
    * Pass NULL, not my plan node, because we don't want anything in this
    118-
    * transient state linking into permanent state. The only possibility
    119-
    * is a SubPlan, and there shouldn't be any (any subselects in the
    120-
    * VALUES list should be InitPlans).
    118+
    * The expressions might contain SubPlans (this is currently only
    119+
    * possible if there's a sub-select containing a LATERAL reference,
    120+
    * otherwise sub-selects in a VALUES list should be InitPlans). Those
    121+
    * subplans will want to hook themselves into our subPlan list, which
    122+
    * would result in a corrupted list after we delete the eval state. We
    123+
    * can work around this by saving and restoring the subPlan list.
    124+
    * (There's no need for the functionality that would be enabled by
    125+
    * having the list entries, since the SubPlans aren't going to be
    126+
    * re-executed anyway.)
    121127
    */
    122-
    exprstatelist = ExecInitExprList(exprlist, NULL);
    128+
    oldsubplans = node->ss.ps.subPlan;
    129+
    node->ss.ps.subPlan = NIL;
    130+
    131+
    exprstatelist = ExecInitExprList(exprlist, &node->ss.ps);
    132+
    133+
    node->ss.ps.subPlan = oldsubplans;
    123134

    124135
    /* parser should have checked all sublists are the same length */
    125136
    Assert(list_length(exprstatelist) == slot->tts_tupleDescriptor->natts);

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

    Lines changed: 30 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -840,6 +840,36 @@ select exists(select * from nocolumns);
    840840
    f
    841841
    (1 row)
    842842

    843+
    --
    844+
    -- Check behavior with a SubPlan in VALUES (bug #14924)
    845+
    --
    846+
    select val.x
    847+
    from generate_series(1,10) as s(i),
    848+
    lateral (
    849+
    values ((select s.i + 1)), (s.i + 101)
    850+
    ) as val(x)
    851+
    where s.i < 10 and (select val.x) < 110;
    852+
    x
    853+
    -----
    854+
    2
    855+
    102
    856+
    3
    857+
    103
    858+
    4
    859+
    104
    860+
    5
    861+
    105
    862+
    6
    863+
    106
    864+
    7
    865+
    107
    866+
    8
    867+
    108
    868+
    9
    869+
    109
    870+
    10
    871+
    (17 rows)
    872+
    843873
    --
    844874
    -- Check sane behavior with nested IN SubLinks
    845875
    --

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

    Lines changed: 10 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -472,6 +472,16 @@ explain (verbose, costs off)
    472472
    create temp table nocolumns();
    473473
    select exists(select * from nocolumns);
    474474

    475+
    --
    476+
    -- Check behavior with a SubPlan in VALUES (bug #14924)
    477+
    --
    478+
    select val.x
    479+
    from generate_series(1,10) as s(i),
    480+
    lateral (
    481+
    values ((select s.i + 1)), (s.i + 101)
    482+
    ) as val(x)
    483+
    where s.i < 10 and (select val.x) < 110;
    484+
    475485
    --
    476486
    -- Check sane behavior with nested IN SubLinks
    477487
    --

    0 commit comments

    Comments
     (0)
    0