diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2017-02-02 19:11:27 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2017-02-02 19:11:27 -0500 |
commit | b971a98cea988e03054077db613fc893564f7bf7 (patch) | |
tree | cdabc3b3b4d63314cbc4572bb48333357c2759d5 | |
parent | 3aab31bbc74b4898d62b83868be5f47215cd36f7 (diff) | |
download | postgresql-b971a98cea988e03054077db613fc893564f7bf7.tar.gz postgresql-b971a98cea988e03054077db613fc893564f7bf7.zip |
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
-rw-r--r-- | src/backend/optimizer/plan/createplan.c | 10 | ||||
-rw-r--r-- | src/backend/optimizer/plan/planner.c | 16 | ||||
-rw-r--r-- | src/test/regress/expected/subselect.out | 23 | ||||
-rw-r--r-- | src/test/regress/sql/subselect.sql | 5 |
4 files changed, 39 insertions, 15 deletions
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 47158f64680..427eedea91e 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -5629,6 +5629,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 d9e2b45065e..23a9e7ba465 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -305,21 +305,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 0fc93d9d726..21fbbb8e222 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -221,6 +221,29 @@ from int8_tbl group by q1 order by q1; 4567890123456789 | 0.6 (2 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 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 29912230891..6e81ffe13f3 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -92,6 +92,11 @@ 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; +-- 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 -- |