BUG #15336: Wrong cursor's bacward fetch results in select with ALL(subquery)
The following bug has been logged on the website:
Bug reference: 15336
Logged by: Vladimir Baranoff
Email address: v.g.baranoff@gmail.com
PostgreSQL version: 10.0
Operating system: Ubuntu 18.04
Description:
create table longs (value int8 ) with ( oids=false );
insert into longs (value) values (1), (2), (3), (4), (5), (6), (7), (8),
(9);
Now fetch results with the cursor in forward direction:
begin;
declare "mycursor" binary scroll cursor for
select "l".value as "column1"
from longs as "l"
where "l".value <> all( select 5 union all select 6 union all select 7)
;
move absolute 0 in "mycursor";
fetch forward 9 from "mycursor";
commit;
The result set is correct (1, 2, 3, 4, 8, 9).
Then execute the following script:
begin;
declare "mycursor" binary scroll cursor for
select "l".value as "column1"
from longs as "l"
where "l".value <> all ( select 5 union all select 6 union all select 7
)
;
move absolute 10 in "mycursor";
fetch backward 9 from "mycursor";
commit;
The result set is wrong (9, 8, 7, 6, 5, 4, 3, 2, 1).
It seems that selection predicate is ignored.
Replacing ALL(subquery) by NOT IN (subquery) solves the problem, but this
violates statement that "NOT IN is equivalent to <> ALL.".
This bug has been reproduced with PostgreSQL 9.6 and 10.0
"PG" == PG Bug reporting form <noreply@postgresql.org> writes:
PG> fetch backward 9 from "mycursor";
PG> commit;
PG> The result set is wrong (9, 8, 7, 6, 5, 4, 3, 2, 1).
PG> It seems that selection predicate is ignored.
PG> This bug has been reproduced with PostgreSQL 9.6 and 10.0
I wonder if we have a contender here for the oldest reported bug in PG
history; while I haven't tested anything older than 9.5, the incorrect
logic seems to date back to the introduction of subqueries in
6.something.
Here is a simpler test case:
begin;
declare foo cursor for select * from generate_series(1,3) i where i <> all (values (2));
fetch all from foo; -- returns the expected 2 rows
fetch backward all from foo; -- assertion failure, or incorrect result
The problem is that the scan direction is being set to "backward" in the
EState, and as a result the subquery evaluation is run in the backward
direction too, which obviously doesn't do anything sensible. The
assertion failure is from the tuplestore code complaining about doing a
backward fetch on a tuplestore not initialized for backward access.
I'm really not sure yet what the correct fix is, though.
--
Andrew (irc:RhodiumToad)
"Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
Andrew> I'm really not sure yet what the correct fix is, though.
I'm guessing that locally saving/restoring the scan direction in
ExecSubPlan is going to be the best option; it seems to fix the problem,
I'll post a patch in a bit.
--
Andrew (irc:RhodiumToad)
"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:
dirfix.patchtext/x-patchDownload+57-2
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
"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.
Looks sane to me ... and a bit astonishing that we didn't run into
this a decade or two back.
regards, tom lane
On 2018-Aug-17, Andrew Gierth wrote:
I wonder if we have a contender here for the oldest reported bug in PG
history; while I haven't tested anything older than 9.5, the incorrect
logic seems to date back to the introduction of subqueries in
6.something.
Hmm ..
begin;
declare foo cursor for select * from generate_series(1,3) i where i <> all (values (2));
fetch all from foo; -- returns the expected 2 rows
fetch backward all from foo; -- assertion failure, or incorrect result
8.2 seems fine:
alvherre=# show debug_assertions;
debug_assertions
------------------
on
(1 fila)
alvherre=# begin;
BEGIN
alvherre=# declare foo cursor for select * from generate_series(1,3) i where i <> all (values (2));
DECLARE CURSOR
alvherre=# fetch all from foo;
i
---
1
3
(2 filas)
alvherre=# fetch backward all from foo;
i
---
3
1
(2 filas)
9.1 does fail an assertion:
TRAP: FailedAssertion(�!(forward || (readptr->eflags & 0x0004))�, Archivo: �/pgsql/source/REL9_1_STABLE/src/backend/utils/sort/tuplestore.c�, L�nea: 765)
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
"Alvaro" == Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I wonder if we have a contender here for the oldest reported bug in
PG history; while I haven't tested anything older than 9.5, the
incorrect logic seems to date back to the introduction of subqueries
in 6.something.
Alvaro> Hmm ..
Alvaro> 8.2 seems fine:
Hah. You're right; the bug is only 10 years old, not 20. It was
apparently introduced in 8.3 by commit c7ff7663e; before that, SubPlans
had a separate EState from the parent plan, so they didn't share the
parent plan's direction indicator.
--
Andrew (irc:RhodiumToad)
"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
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.
Tom> Looks sane to me ...
Pushed.
--
Andrew (irc:RhodiumToad)