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)
Date: Fri, 17 Aug 2018 14:56:46 +0100
Message-ID: <87in492jrn.fsf@news-spur.riddles.org.uk> (raw)
In-Reply-To: <87mutl2pv1.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>
>>>>> "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)
Attachments:
[text/x-patch] dirfix.patch (3.5K, ../87in492jrn.fsf@news-spur.riddles.org.uk/2-dirfix.patch)
download | inline diff:diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.cindex 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.outindex 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.sqlindex 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;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: andrew@tao11.riddles.org.uk, pgsql-bugs@lists.postgresql.org, v.g.baranoff@gmail.com
Subject: Re: BUG #15336: Wrong cursor's bacward fetch results in select with ALL(subquery)
In-Reply-To: <87in492jrn.fsf@news-spur.riddles.org.uk>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox