From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: pgsql-bugs@lists.postgresql.org
Cc: v.g.baranoff@gmail.com
Subject: Re: BUG #15336: Wrong cursor's bacward fetch results in select with
 ALL(subquery)
In-Reply-To: <87mutl2pv1.fsf@news-spur.riddles.org.uk> (Andrew Gierth's
	message of "Fri, 17 Aug 2018 12:49:19 +0100")
Message-ID: <87in492jrn.fsf@news-spur.riddles.org.uk>
References: <153449812167.1304.1741624125628126322@wrigleys.postgresql.org>
	<87va892tf5.fsf@news-spur.riddles.org.uk>
	<87mutl2pv1.fsf@news-spur.riddles.org.uk>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (berkeley-unix)
Date: Fri, 17 Aug 2018 14:56:46 +0100
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Precedence: bulk

--=-=-=
Content-Type: text/plain

>>>>> "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)


--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline; filename=dirfix.patch
Content-Description: subplan scan dir fix

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;

--=-=-=--