Thread: Re: [GENERAL] Postgres 7.2 - Updating rows in cursor problem

Vaclav Kulakovsky <vaclav.kulakovsky@definity.cz> writes:
> I've a problem in PG 7.2. If you update rows which are included in plpgsql
> RECORD , updated rows are again added to the RECORD, so you will get into
> infinite loop.

This is a bug in plgsql, or more precisely in SPI, I think.  The FOR
statement needs to restore its initial value of scanCommandId each time
it resumes execution of the SELECT.  Seems like that should be done down
inside SPI.  Comments?
        regards, tom lane

I wrote:
> This is a bug in plgsql, or more precisely in SPI, I think.  The FOR
> statement needs to restore its initial value of scanCommandId each time
> it resumes execution of the SELECT.  Seems like that should be done down
> inside SPI.  Comments?

More specifically, the problem is that plpgsql's FOR-over-a-select now
depends on a SPI cursor, and both SPI cursors and regular cursors are
broken in this regard.  Observe the following misbehavior with a plain
cursor:

regression=# select * from foo;f1 | f2
----+---- 1 |  1 2 |  2 3 |  3
(3 rows)

regression=# begin;
BEGIN
regression=# declare c cursor for select * from foo;
SELECT
regression=# fetch 2 from c;f1 | f2
----+---- 1 |  1 2 |  2
(2 rows)

regression=# update foo set f2 = f2 + 1;
UPDATE 3
regression=# fetch all from c;f1 | f2
----+---- 1 |  2 2 |  3 3 |  4
(3 rows)

IMHO the cursor should not be able to see the rows inserted by the
subsequent UPDATE.  (Certainly it should not return the updated versions
of rows it's already returned.)  The SQL spec says that cursors declared
INSENSITIVE shall not observe changes made after they are opened --- and
it gives the implementation the option to make all cursors behave that
way.  I think we should choose to do so.

I believe the correct fix for this is that Portal objects should store
the scanCommandId that was current when they were created, and restore
this scanCommandId whenever they are asked to run their plan.  Comments?
        regards, tom lane

> -----Original Message-----
> From:  Tom Lane
>
> I wrote:
> > This is a bug in plgsql, or more precisely in SPI, I think.  The FOR
> > statement needs to restore its initial value of scanCommandId each time
> > it resumes execution of the SELECT.  Seems like that should be done down
> > inside SPI.  Comments?
>
> More specifically, the problem is that plpgsql's FOR-over-a-select now
> depends on a SPI cursor, and both SPI cursors and regular cursors are
> broken in this regard.  Observe the following misbehavior with a plain
> cursor:

This is a known issue. We should implement INSENSITIVE cursors
to avoid this behavior. The keyword INSENSITIVE is there but isn't
used long. I plan to implement this feature as the first step toward
cross transaction cursors. Saving the xid and commandid in the
portal or snapshot and restoring them at fetch(move) time would
solve it.

regards,
Hiroshi Inoue

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
> This is a known issue. We should implement INSENSITIVE cursors
> to avoid this behavior. The keyword INSENSITIVE is there but isn't
> used long. I plan to implement this feature as the first step toward
> cross transaction cursors. Saving the xid and commandid in the
> portal or snapshot and restoring them at fetch(move) time would
> solve it.

For the moment I've arranged to save commandId in portals.  (xid isn't
needed since we don't have cross-transaction portals ... yet)

It occurs to me though that scanCommandId should not be part of the
xact.c global status at all.  It should be stored in heapscan and
indexscan state structs, instead.  I have been thinking about trying
to clean up the API for heapscans and indexscans, and maybe I'll see
if that can be done as part of that work.
        regards, tom lane