aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2017-02-02 19:11:27 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2017-02-02 19:11:27 -0500
commitb971a98cea988e03054077db613fc893564f7bf7 (patch)
treecdabc3b3b4d63314cbc4572bb48333357c2759d5
parent3aab31bbc74b4898d62b83868be5f47215cd36f7 (diff)
downloadpostgresql-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.c10
-rw-r--r--src/backend/optimizer/plan/planner.c16
-rw-r--r--src/test/regress/expected/subselect.out23
-rw-r--r--src/test/regress/sql/subselect.sql5
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
--