Fix placement of initPlans when forcibly materializing a subplan.

If we forcibly place a Material node atop a finished subplan, we need
to move any initPlans attached to the subplan up to the Material node,
in order to keep SS_finalize_plan() happy.  I'd figured this out in
commit 7b67a0a49 for the case of materializing a cursor plan, but out of
an abundance of caution, I put the initPlan movement hack at the call
site for that case, rather than inside materialize_finished_plan().
That was the wrong thing, because it turns out to also be necessary for
the only other caller of materialize_finished_plan(), ie subselect.c.
We lacked any test cases that exposed the mistake, but bug#14524 from
Wei Congrui shows that it's possible to get an initPlan reference into
the top tlist in that case too, and then SS_finalize_plan() complains.
Hence, move the hack into materialize_finished_plan().

In HEAD, also relocate some recently-added tests in subselect.sql, which
I'd unthinkingly dropped into the middle of a sequence of related tests.

Report: https://postgr.es/m/20170202060020.1400.89021@wrigleys.postgresql.org

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index fae1f67b9c041b8f44e34fb24302a98396de5ecc..997bdcff2ea90f0dcc1e66b1e8346f9c2bb3652c 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -5704,6 +5704,16 @@ materialize_finished_plan(Plan *subplan)
 
        matplan = (Plan *) make_material(subplan);
 
+       /*
+        * XXX horrid kluge: if there are any initPlans attached to the subplan,
+        * move them up to the Material node, which is now effectively the top
+        * plan node in its query level.  This prevents failure in
+        * SS_finalize_plan(), which see for comments.  We don't bother adjusting
+        * the subplan's cost estimate for this.
+        */
+       matplan->initPlan = subplan->initPlan;
+       subplan->initPlan = NIL;
+
        /* Set cost data */
        cost_material(&matpath,
                                  subplan->startup_cost,

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 4b5902fc3ecbe0dea67b3a6363cdf0ea81b7f24a..881742f46b66d7cfcdf5161f93277a57ff39307c 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -316,21 +316,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
        if (cursorOptions & CURSOR_OPT_SCROLL)
        {
                if (!ExecSupportsBackwardScan(top_plan))
-               {
-                       Plan       *sub_plan = top_plan;
-
-                       top_plan = materialize_finished_plan(sub_plan);
-
-                       /*
-                        * XXX horrid kluge: if there are any initPlans attached to the
-                        * formerly-top plan node, move them up to the Material node. This
-                        * prevents failure in SS_finalize_plan, which see for comments.
-                        * We don't bother adjusting the sub_plan's cost estimate for
-                        * this.
-                        */
-                       top_plan->initPlan = sub_plan->initPlan;
-                       sub_plan->initPlan = NIL;
-               }
+                       top_plan = materialize_finished_plan(top_plan);
        }
 
        /*

diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index 1ad4ad47b2c011c770f8ac823d05e04f16c63952..ed7d6d8034e302362d12c0d9d00926ba0d3ba94f 100644 (file)
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -196,6 +196,31 @@ SELECT '' AS five, f1 AS "Correlated Field"
       |                3
 (5 rows)
 
+--
+-- Use some existing tables in the regression test
+--
+SELECT '' AS eight, ss.f1 AS "Correlated Field", ss.f3 AS "Second Field"
+  FROM SUBSELECT_TBL ss
+  WHERE f1 NOT IN (SELECT f1+1 FROM INT4_TBL
+                   WHERE f1 != ss.f1 AND f1 < 2147483647);
+ eight | Correlated Field | Second Field 
+-------+------------------+--------------
+       |                2 |            4
+       |                3 |            5
+       |                2 |            2
+       |                3 |            3
+       |                6 |            8
+       |                8 |             
+(6 rows)
+
+select q1, float8(count(*)) / (select count(*) from int8_tbl)
+from int8_tbl group by q1 order by q1;
+        q1        | ?column? 
+------------------+----------
+              123 |      0.4
+ 4567890123456789 |      0.6
+(2 rows)
+
 -- Unspecified-type literals in output columns should resolve as text
 SELECT *, pg_typeof(f1) FROM
   (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1;
@@ -227,30 +252,28 @@ explain verbose select '42' union all select 43;
          Output: 43
 (5 rows)
 
---
--- Use some existing tables in the regression test
---
-SELECT '' AS eight, ss.f1 AS "Correlated Field", ss.f3 AS "Second Field"
-  FROM SUBSELECT_TBL ss
-  WHERE f1 NOT IN (SELECT f1+1 FROM INT4_TBL
-                   WHERE f1 != ss.f1 AND f1 < 2147483647);
- eight | Correlated Field | Second Field 
--------+------------------+--------------
-       |                2 |            4
-       |                3 |            5
-       |                2 |            2
-       |                3 |            3
-       |                6 |            8
-       |                8 |             
-(6 rows)
+-- check materialization of an initplan reference (bug #14524)
+explain (verbose, costs off)
+select 1 = all (select (select 1));
+            QUERY PLAN             
+-----------------------------------
+ Result
+   Output: (SubPlan 2)
+   SubPlan 2
+     ->  Materialize
+           Output: ($0)
+           InitPlan 1 (returns $0)
+             ->  Result
+                   Output: 1
+           ->  Result
+                 Output: $0
+(10 rows)
 
-select q1, float8(count(*)) / (select count(*) from int8_tbl)
-from int8_tbl group by q1 order by q1;
-        q1        | ?column? 
-------------------+----------
-              123 |      0.4
- 4567890123456789 |      0.6
-(2 rows)
+select 1 = all (select (select 1));
+ ?column? 
+----------
+ t
+(1 row)
 
 --
 -- Check EXISTS simplification with LIMIT

diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 9c2a73d4d7768055884c3a1755952f28a086484e..2fc0e26ca066a97c67aea259231b8f5bf7d7cb19 100644 (file)
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -80,16 +80,6 @@ SELECT '' AS five, f1 AS "Correlated Field"
   WHERE (f1, f2) IN (SELECT f2, CAST(f3 AS int4) FROM SUBSELECT_TBL
                      WHERE f3 IS NOT NULL);
 
--- Unspecified-type literals in output columns should resolve as text
-
-SELECT *, pg_typeof(f1) FROM
-  (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1;
-
--- ... unless there's context to suggest differently
-
-explain verbose select '42' union all select '43';
-explain verbose select '42' union all select 43;
-
 --
 -- Use some existing tables in the regression test
 --
@@ -102,6 +92,21 @@ SELECT '' AS eight, ss.f1 AS "Correlated Field", ss.f3 AS "Second Field"
 select q1, float8(count(*)) / (select count(*) from int8_tbl)
 from int8_tbl group by q1 order by q1;
 
+-- Unspecified-type literals in output columns should resolve as text
+
+SELECT *, pg_typeof(f1) FROM
+  (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1;
+
+-- ... unless there's context to suggest differently
+
+explain verbose select '42' union all select '43';
+explain verbose select '42' union all select 43;
+
+-- check materialization of an initplan reference (bug #14524)
+explain (verbose, costs off)
+select 1 = all (select (select 1));
+select 1 = all (select (select 1));
+
 --
 -- Check EXISTS simplification with LIMIT
 --