Обсуждение: Bug #866: Cursor scrolling broken in 7.3.1 (works in 7.2.1)

Florian Wunderlich (fwunderlich@devbrain.de) reports a bug with a severity of 1
The lower the number the more severe it is.

Short Description
Cursor scrolling broken in 7.3.1 (works in 7.2.1)

Long Description
A MOVE BACKWARD after a FETCH in a cursor declared for a SELECT with a WHERE clause does not work. The cursor stays
positionedwhere it is. 

In the example code, it is expected that both FETCH statements return the same tuples (one tuple with id=1).

This is the case with postgresql-7.2.1, started with exactly the same options as 7.3.1, which does not return any
tuplesfor the second FETCH statement. 7.3.1 does however work correctly if the WHERE clause is dropped. 

There seems to be some correlation with bug 664 / 665, which does not seem to have been fixed.


Here is the example code pasted to psql connected to 7.3.1:

items=# begin;
BEGIN
items=# create table test (id int4 primary key);
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index 'test_pkey' for table 'test'
CREATE TABLE
items=# insert into test values (1);
INSERT 31047 1
items=# insert into test values (2);
INSERT 31048 1
items=# declare c scroll cursor for select * from test where (id = 1);
DECLARE CURSOR
items=# fetch all from c;
 id
----
  1
(1 row)

items=# move backward all in c;
MOVE 0
items=# fetch all from c;
 id
----
(0 rows)

items=# rollback;
ROLLBACK
items=# \q


Here is the correct 7.2.1 behavior:

items=# begin;
BEGIN
items=# create table test (id int4 primary key);
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index 'test_pkey' for table 'test'
CREATE
items=# insert into test values (1);
INSERT 197564 1
items=# insert into test values (2);
INSERT 197565 1
items=# declare c scroll cursor for select * from test where (id = 1);
DECLARE
items=# fetch all from c;
 id
----
  1
(1 row)

items=# move backward all in c;
MOVE 1
items=# fetch all from c;
 id
----
  1
(1 row)

items=# rollback;
ROLLBACK
items=# \q


Sample Code
begin;
create table test (id int4 primary key);
insert into test values (1);
insert into test values (2);
declare c scroll cursor for select * from test where (id = 1);
fetch all from c;
move backward all in c;
fetch all from c;
rollback;


No file was uploaded with this report

pgsql-bugs@postgresql.org writes:
> Cursor scrolling broken in 7.3.1 (works in 7.2.1)

That's a fairly sweeping claim.

I have found a bug that explains this problem for the case where the
cursor retrieves a single row using a unique index.  Have you seen it
happen in any other cases?

            regards, tom lane

If you need it right away, here is the patch for the unique-index
problem.  This will be in 7.3.2.

            regards, tom lane


*** src/backend/access/index/genam.c.orig    Wed Sep  4 17:30:07 2002
--- src/backend/access/index/genam.c    Wed Jan  8 14:25:44 2003
***************
*** 107,112 ****
--- 107,115 ----
      /* mark cached function lookup data invalid; it will be set later */
      scan->fn_getnext.fn_oid = InvalidOid;

+     scan->unique_tuple_pos = 0;
+     scan->unique_tuple_mark = 0;
+
      pgstat_initstats(&scan->xs_pgstat_info, indexRelation);

      /*
*** src/backend/access/index/indexam.c.orig    Wed Sep  4 17:30:08 2002
--- src/backend/access/index/indexam.c    Wed Jan  8 14:34:21 2003
***************
*** 308,313 ****
--- 308,315 ----
      scan->kill_prior_tuple = false;        /* for safety */
      scan->keys_are_unique = false;        /* may be set by amrescan */
      scan->got_tuple = false;
+     scan->unique_tuple_pos = 0;
+     scan->unique_tuple_mark = 0;

      OidFunctionCall2(procedure,
                       PointerGetDatum(scan),
***************
*** 360,365 ****
--- 362,369 ----
      SCAN_CHECKS;
      GET_SCAN_PROCEDURE(markpos, ammarkpos);

+     scan->unique_tuple_mark = scan->unique_tuple_pos;
+
      OidFunctionCall1(procedure, PointerGetDatum(scan));
  }

***************
*** 376,382 ****
      GET_SCAN_PROCEDURE(restrpos, amrestrpos);

      scan->kill_prior_tuple = false;        /* for safety */
!     scan->got_tuple = false;

      OidFunctionCall1(procedure, PointerGetDatum(scan));
  }
--- 380,392 ----
      GET_SCAN_PROCEDURE(restrpos, amrestrpos);

      scan->kill_prior_tuple = false;        /* for safety */
!
!     /*
!      * We do not reset got_tuple; so if the scan is actually being
!      * short-circuited by index_getnext, the effective position restoration
!      * is done by restoring unique_tuple_pos.
!      */
!     scan->unique_tuple_pos = scan->unique_tuple_mark;

      OidFunctionCall1(procedure, PointerGetDatum(scan));
  }
***************
*** 398,403 ****
--- 408,439 ----

      SCAN_CHECKS;

+     /*
+      * Can skip entering the index AM if we already got a tuple and it
+      * must be unique.  Instead, we need a "short circuit" path that
+      * just keeps track of logical scan position (before/on/after tuple).
+      *
+      * Note that we hold the pin on the single tuple's buffer throughout
+      * the scan once we are in this state.
+      */
+     if (scan->keys_are_unique && scan->got_tuple)
+     {
+         if (ScanDirectionIsForward(direction))
+         {
+             if (scan->unique_tuple_pos <= 0)
+                 scan->unique_tuple_pos++;
+         }
+         else if (ScanDirectionIsBackward(direction))
+         {
+             if (scan->unique_tuple_pos >= 0)
+                 scan->unique_tuple_pos--;
+         }
+         if (scan->unique_tuple_pos == 0)
+             return heapTuple;
+         else
+             return NULL;
+     }
+
      /* Release any previously held pin */
      if (BufferIsValid(scan->xs_cbuf))
      {
***************
*** 408,420 ****
      /* just make sure this is false... */
      scan->kill_prior_tuple = false;

-     /*
-      * Can skip entering the index AM if we already got a tuple and it
-      * must be unique.
-      */
-     if (scan->keys_are_unique && scan->got_tuple)
-         return NULL;
-
      for (;;)
      {
          bool        found;
--- 444,449 ----
***************
*** 474,479 ****
--- 503,514 ----

      /* Success exit */
      scan->got_tuple = true;
+
+     /*
+      * If we just fetched a known-unique tuple, then subsequent calls will
+      * go through the short-circuit code above.  unique_tuple_pos has been
+      * initialized to 0, which is the correct state ("on row").
+      */

      pgstat_count_index_getnext(&scan->xs_pgstat_info);

*** src/include/access/relscan.h.orig    Wed Sep  4 17:31:07 2002
--- src/include/access/relscan.h    Wed Jan  8 14:11:13 2003
***************
*** 70,75 ****
--- 70,84 ----

      FmgrInfo    fn_getnext;        /* cached lookup info for AM's getnext fn */

+     /*
+      * If keys_are_unique and got_tuple are both true, we stop calling the
+      * index AM; it is then necessary for index_getnext to keep track of
+      * the logical scan position for itself.  It does that using
+      * unique_tuple_pos: -1 = before row, 0 = on row, +1 = after row.
+      */
+     int            unique_tuple_pos; /* logical position */
+     int            unique_tuple_mark; /* logical marked position */
+
      PgStat_Info xs_pgstat_info; /* statistics collector hook */
  } IndexScanDescData;

Tom Lane wrote:
>
> pgsql-bugs@postgresql.org writes:
> > Cursor scrolling broken in 7.3.1 (works in 7.2.1)
>
> That's a fairly sweeping claim.
>
> I have found a bug that explains this problem for the case where the
> cursor retrieves a single row using a unique index.  Have you seen it
> happen in any other cases?
>
>                         regards, tom lane

That was not meant as a claim, but only as a help to classify the bug -
something is broken there. The text from the "long description" field of
the bug report form describes what the problem actually is.

I have checked the posted example modified for a non-unique index and
the bug does not occur. It did not occur for me under any other
circumstances yet.