Horton

Thread

Commits

  1. Set scan direction appropriately for SubPlans (bug #15336)

  2. Get rid of the separate EState for subplans, and just let them share the

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

    The Post Office <noreply@postgresql.org> — 2018-08-17T09:28:41Z

    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

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

    Andrew Gierth <andrew@tao11.riddles.org.uk> — 2018-08-17T10:27:46Z

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

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

    Andrew Gierth <andrew@tao11.riddles.org.uk> — 2018-08-17T11:49:19Z

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

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

    Andrew Gierth <andrew@tao11.riddles.org.uk> — 2018-08-17T13:56:46Z

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

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-17T13:59:16Z

    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

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

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-08-17T14:04:11Z

    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

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

    Andrew Gierth <andrew@tao11.riddles.org.uk> — 2018-08-17T14:38:41Z

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

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

    Andrew Gierth <andrew@tao11.riddles.org.uk> — 2018-08-17T15:44:46Z

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