Re: BUG #15336: Wrong cursor's bacward fetch results in select with ALL(subquery)

>>>>> "Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

 Andrew> I'm guessing that locally saving/restoring the scan direction
 Andrew> in ExecSubPlan is going to be the best option; it seems to fix
 Andrew> the problem, I'll post a patch in a bit.

It turns out to be also necessary to do this in ExecSetParamPlan, though
I couldn't find a way to make a stable regression test for that - my
manual tests were based on putting a subselect inside a volatile
construct like CASE WHEN random() < x THEN.

Patch attached.

-- 
Andrew (irc:RhodiumToad)

diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index 44f551bcf1..6b370750c5 100644
--- 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 2904ae43e5..588d069589 100644
--- 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 9b7125c111..843f511b3d 100644
--- 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;