diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 71ed6a488035dc72dd1961c4f572b50d011a0c75..cebfe1973460b88ac67b0b53c324ddb349c2be4b 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -314,9 +314,10 @@ create_plan(PlannerInfo *root, Path *best_path)
/*
* Attach any initPlans created in this query level to the topmost plan
- * node. (The initPlans could actually go in any plan node at or above
- * where they're referenced, but there seems no reason to put them any
- * lower than the topmost node for the query level.)
+ * node. (In principle the initplans could go in any plan node at or
+ * above where they're referenced, but there seems no reason to put them
+ * any lower than the topmost node for the query level. Also, see
+ * comments for SS_finalize_plan before you try to change this.)
*/
SS_attach_initplans(root, plan);
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 0ece4f2aaceef76b2eca362d3b366e084c28a2f8..a66317367c3435a5b92b8179e7732054812c1d8d 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -305,15 +305,33 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
if (cursorOptions & CURSOR_OPT_SCROLL)
{
if (!ExecSupportsBackwardScan(top_plan))
- top_plan = materialize_finished_plan(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;
+ }
}
/*
* Optionally add a Gather node for testing purposes, provided this is
- * actually a safe thing to do.
+ * actually a safe thing to do. (Note: we assume adding a Material node
+ * above did not change the parallel safety of the plan, so we can still
+ * rely on best_path->parallel_safe. However, that flag doesn't account
+ * for initPlans, which render the plan parallel-unsafe.)
*/
- if (best_path->parallel_safe &&
- force_parallel_mode != FORCE_PARALLEL_OFF)
+ if (force_parallel_mode != FORCE_PARALLEL_OFF &&
+ best_path->parallel_safe &&
+ top_plan->initPlan == NIL)
{
Gather *gather = makeNode(Gather);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 0849b1d56346cd3fec5a8143e36fb234842ea2a6..a46cc108203706e393465c5892f2cb1f6f73a6d1 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -2187,7 +2187,7 @@ SS_charge_for_initplans(PlannerInfo *root, RelOptInfo *final_rel)
*
* Attach any initplans created in the current query level to the specified
* plan node, which should normally be the topmost node for the query level.
- * (The initPlans could actually go in any node at or above where they're
+ * (In principle the initPlans could go in any node at or above where they're
* referenced; but there seems no reason to put them any lower than the
* topmost node, so we don't bother to track exactly where they came from.)
* We do not touch the plan node's cost; the initplans should have been
@@ -2226,9 +2226,22 @@ SS_finalize_plan(PlannerInfo *root, Plan *plan)
* recursion.
*
* The return value is the computed allParam set for the given Plan node.
- * This is just an internal notational convenience.
+ * This is just an internal notational convenience: we can add a child
+ * plan's allParams to the set of param IDs of interest to this level
+ * in the same statement that recurses to that child.
*
* Do not scribble on caller's values of valid_params or scan_params!
+ *
+ * Note: although we attempt to deal with initPlans anywhere in the tree, the
+ * logic is not really right. The problem is that a plan node might return an
+ * output Param of its initPlan as a targetlist item, in which case it's valid
+ * for the parent plan level to reference that same Param; the parent's usage
+ * will be converted into a Var referencing the child plan node by setrefs.c.
+ * But this function would see the parent's reference as out of scope and
+ * complain about it. For now, this does not matter because the planner only
+ * attaches initPlans to the topmost plan node in a query level, so the case
+ * doesn't arise. If we ever merge this processing into setrefs.c, maybe it
+ * can be handled more cleanly.
*/
static Bitmapset *
finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index 462ad231c368d294045bafdd4bc847493b55cbe0..3ae918a63c5240e472fbb7668f1e2fca97d30cb4 100644
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1285,3 +1285,68 @@ fetch all from c;
(3 rows)
rollback;
+-- Check handling of non-backwards-scan-capable plans with scroll cursors
+begin;
+explain (costs off) declare c1 cursor for select (select 42) as x;
+ QUERY PLAN
+---------------------------
+ Result
+ InitPlan 1 (returns $0)
+ -> Result
+(3 rows)
+
+explain (costs off) declare c1 scroll cursor for select (select 42) as x;
+ QUERY PLAN
+---------------------------
+ Materialize
+ InitPlan 1 (returns $0)
+ -> Result
+ -> Result
+(4 rows)
+
+declare c1 scroll cursor for select (select 42) as x;
+fetch all in c1;
+ x
+----
+ 42
+(1 row)
+
+fetch backward all in c1;
+ x
+----
+ 42
+(1 row)
+
+rollback;
+begin;
+explain (costs off) declare c2 cursor for select generate_series(1,3) as g;
+ QUERY PLAN
+------------
+ Result
+(1 row)
+
+explain (costs off) declare c2 scroll cursor for select generate_series(1,3) as g;
+ QUERY PLAN
+--------------
+ Materialize
+ -> Result
+(2 rows)
+
+declare c2 scroll cursor for select generate_series(1,3) as g;
+fetch all in c2;
+ g
+---
+ 1
+ 2
+ 3
+(3 rows)
+
+fetch backward all in c2;
+ g
+---
+ 3
+ 2
+ 1
+(3 rows)
+
+rollback;
diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql
index 01c3b85da9a763697fd4e5eac3f5984f24e88c58..a1c812e937f78a5d45974d97ee7f9233af0dc6c0 100644
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -484,3 +484,19 @@ fetch all from c;
move backward all in c;
fetch all from c;
rollback;
+
+-- Check handling of non-backwards-scan-capable plans with scroll cursors
+begin;
+explain (costs off) declare c1 cursor for select (select 42) as x;
+explain (costs off) declare c1 scroll cursor for select (select 42) as x;
+declare c1 scroll cursor for select (select 42) as x;
+fetch all in c1;
+fetch backward all in c1;
+rollback;
+begin;
+explain (costs off) declare c2 cursor for select generate_series(1,3) as g;
+explain (costs off) declare c2 scroll cursor for select generate_series(1,3) as g;
+declare c2 scroll cursor for select generate_series(1,3) as g;
+fetch all in c2;
+fetch backward all in c2;
+rollback;