Set scan direction appropriately for SubPlans (bug #15336)

When executing a SubPlan in an expression, the EState's direction
field was left alone, resulting in an attempt to execute the subplan
backwards if it was encountered during a backwards scan of a cursor.
Also, though much less likely, it was possible to reach the execution
of an InitPlan while in backwards-scan state.

Repair by saving/restoring estate->es_direction and forcing forward
scan mode in the relevant places.

Backpatch all the way, since this has been broken since 8.3 (prior to
commit c7ff7663e, SubPlans had their own EStates rather than sharing
the parent plan's, so there was no confusion over scan direction).

Per bug #15336 reported by Vladimir Baranoff; analysis and patch by
me, review by Tom Lane.

Discussion: https://postgr.es/m/153449812167.1304.1741624125628126322@wrigleys.postgresql.org

diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index 44f551bcf1f61d865211cdcd5b503faef1ac2627..6b370750c5ba1afc174453fc897321b00faaee3b 100644 (file)
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -65,6 +65,9 @@ ExecSubPlan(SubPlanState *node,
                        bool *isNull)
 {
        SubPlan    *subplan = node->subplan;
+       EState     *estate = node->planstate->state;
+       ScanDirection dir = estate->es_direction;
+       Datum           retval;
 
        CHECK_FOR_INTERRUPTS();
 
@@ -77,11 +80,19 @@ ExecSubPlan(SubPlanState *node,
        if (subplan->setParam != NIL && subplan->subLinkType != MULTIEXPR_SUBLINK)
                elog(ERROR, "cannot set parent params from subquery");
 
+       /* Force forward-scan mode for evaluation */
+       estate->es_direction = ForwardScanDirection;
+
        /* Select appropriate evaluation strategy */
        if (subplan->useHashTable)
-               return ExecHashSubPlan(node, econtext, isNull);
+               retval = ExecHashSubPlan(node, econtext, isNull);
        else
-               return ExecScanSubPlan(node, econtext, isNull);
+               retval = ExecScanSubPlan(node, econtext, isNull);
+
+       /* restore scan direction */
+       estate->es_direction = dir;
+
+       return retval;
 }
 
 /*
@@ -1006,6 +1017,8 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
        SubPlan    *subplan = node->subplan;
        PlanState  *planstate = node->planstate;
        SubLinkType subLinkType = subplan->subLinkType;
+       EState     *estate = planstate->state;
+       ScanDirection dir = estate->es_direction;
        MemoryContext oldcontext;
        TupleTableSlot *slot;
        ListCell   *pvar;
@@ -1019,6 +1032,12 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
        if (subLinkType == CTE_SUBLINK)
                elog(ERROR, "CTE subplans should not be executed via ExecSetParamPlan");
 
+       /*
+        * Enforce forward scan direction regardless of caller. It's hard but not
+        * impossible to get here in backward scan, so make it work anyway.
+        */
+       estate->es_direction = ForwardScanDirection;
+
        /* Initialize ArrayBuildStateAny in caller's context, if needed */
        if (subLinkType == ARRAY_SUBLINK)
                astate = initArrayResultAny(subplan->firstColType,
@@ -1171,6 +1190,9 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
        }
 
        MemoryContextSwitchTo(oldcontext);
+
+       /* restore scan direction */
+       estate->es_direction = dir;
 }
 
 /*

diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index 2904ae43e55f5b6067c6275cf9e4dd18c684d385..588d069589277870eb3bcb9ec526a174d3d111a5 100644 (file)
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -1137,3 +1137,20 @@ select * from (select pk,c2 from sq_limit order by c1,pk) as x limit 3;
 
 drop function explain_sq_limit();
 drop table sq_limit;
+--
+-- Ensure that backward scan direction isn't propagated into
+-- expression subqueries (bug #15336)
+--
+begin;
+declare c1 scroll cursor for
+ select * from generate_series(1,4) i
+  where i <> all (values (2),(3));
+move forward all in c1;
+fetch backward all in c1;
+ i 
+---
+ 4
+ 1
+(2 rows)
+
+commit;

diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 9b7125c111cb2a3453b6b2cfbfe07a6715ccc669..843f511b3dc92201f26b2f4386a64601680df948 100644 (file)
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -609,3 +609,19 @@ select * from (select pk,c2 from sq_limit order by c1,pk) as x limit 3;
 drop function explain_sq_limit();
 
 drop table sq_limit;
+
+--
+-- Ensure that backward scan direction isn't propagated into
+-- expression subqueries (bug #15336)
+--
+
+begin;
+
+declare c1 scroll cursor for
+ select * from generate_series(1,4) i
+  where i <> all (values (2),(3));
+
+move forward all in c1;
+fetch backward all in c1;
+
+commit;