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

diff --git a/src/backend/executor/nodeValuesscan.c b/src/backend/executor/nodeValuesscan.c
index 6eacaed8bb8cf46c7863428f03331ea7e15a85f3..734795dcbf19f885803fceba9a53fb147a4f5d4e 100644 (file)
--- a/src/backend/executor/nodeValuesscan.c
+++ b/src/backend/executor/nodeValuesscan.c
@@ -92,6 +92,7 @@ ValuesNext(ValuesScanState *node)
        if (exprlist)
        {
                MemoryContext oldContext;
+               List       *oldsubplans;
                List       *exprstatelist;
                Datum      *values;
                bool       *isnull;
@@ -115,12 +116,22 @@ ValuesNext(ValuesScanState *node)
                oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
 
                /*
-                * Pass NULL, not my plan node, because we don't want anything in this
-                * transient state linking into permanent state.  The only possibility
-                * is a SubPlan, and there shouldn't be any (any subselects in the
-                * VALUES list should be InitPlans).
+                * The expressions might contain SubPlans (this is currently only
+                * possible if there's a sub-select containing a LATERAL reference,
+                * otherwise sub-selects in a VALUES list should be InitPlans). Those
+                * subplans will want to hook themselves into our subPlan list, which
+                * would result in a corrupted list after we delete the eval state. We
+                * can work around this by saving and restoring the subPlan list.
+                * (There's no need for the functionality that would be enabled by
+                * having the list entries, since the SubPlans aren't going to be
+                * re-executed anyway.)
                 */
-               exprstatelist = ExecInitExprList(exprlist, NULL);
+               oldsubplans = node->ss.ps.subPlan;
+               node->ss.ps.subPlan = NIL;
+
+               exprstatelist = ExecInitExprList(exprlist, &node->ss.ps);
+
+               node->ss.ps.subPlan = oldsubplans;
 
                /* parser should have checked all sublists are the same length */
                Assert(list_length(exprstatelist) == slot->tts_tupleDescriptor->natts);

diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index 873985395c13c9a778113564d3aee228d23fc897..796426c66e22b11f8128b2d684b6aa46a9f91ff7 100644 (file)
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -840,6 +840,36 @@ select exists(select * from nocolumns);
  f
 (1 row)
 
+--
+-- Check behavior with a SubPlan in VALUES (bug #14924)
+--
+select val.x
+  from generate_series(1,10) as s(i),
+  lateral (
+    values ((select s.i + 1)), (s.i + 101)
+  ) as val(x)
+where s.i < 10 and (select val.x) < 110;
+  x  
+-----
+   2
+ 102
+   3
+ 103
+   4
+ 104
+   5
+ 105
+   6
+ 106
+   7
+ 107
+   8
+ 108
+   9
+ 109
+  10
+(17 rows)
+
 --
 -- Check sane behavior with nested IN SubLinks
 --

diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 1b3b6cf1d7dbe175a2ecab0a2481cae00cd89909..15035afd0b0b0aada2631f1ff976b21cdb5ab42f 100644 (file)
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -472,6 +472,16 @@ explain (verbose, costs off)
 create temp table nocolumns();
 select exists(select * from nocolumns);
 
+--
+-- Check behavior with a SubPlan in VALUES (bug #14924)
+--
+select val.x
+  from generate_series(1,10) as s(i),
+  lateral (
+    values ((select s.i + 1)), (s.i + 101)
+  ) as val(x)
+where s.i < 10 and (select val.x) < 110;
+
 --
 -- Check sane behavior with nested IN SubLinks
 --