Re: We don't enforce NO SCROLL cursor restrictions

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: We don't enforce NO SCROLL cursor restrictions
Дата
Msg-id 3725815.1631215280@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: We don't enforce NO SCROLL cursor restrictions  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: We don't enforce NO SCROLL cursor restrictions  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
I wrote:
> [ pokes at it some more ... ]  Hm, we let you do this:
> ...
> which definitely flies in the face of the fact that we disallow
> combining SCROLL and FOR UPDATE:
> regression=*# declare c scroll cursor for select * from int8_tbl for update;
> ERROR:  DECLARE SCROLL CURSOR ... FOR UPDATE is not supported
> DETAIL:  Scrollable cursors must be READ ONLY.
> 
> I don't recall the exact reason for that prohibition, but I wonder
> if there aren't user-visible anomalies reachable from the fact that
> you can bypass it.

I dug in the archives.  The above-quoted error message was added by
me in 048efc25e, responding to Heikki's complaint here:

https://www.postgresql.org/message-id/471F69FE.5000500%40enterprisedb.com

What I now see is that I put that check at the wrong level.  It
successfully blocks off the case Heikki complained of:

DROP TABLE IF EXISTS foo;
CREATE TABLE foo (id integer);
INSERT INTO foo SELECT a from generate_series(1,10) a;
BEGIN;
DECLARE c CURSOR FOR SELECT id FROM foo FOR UPDATE;
FETCH 2 FROM c;
UPDATE foo set ID=20 WHERE CURRENT OF c;
FETCH RELATIVE 0 FROM c;
COMMIT;

The FETCH RELATIVE 0 fails with

ERROR:  cursor can only scan forward
HINT:  Declare it with SCROLL option to enable backward scan.

However, if you replace that with the should-be-equivalent

FETCH ABSOLUTE 2 FROM c;

then what you get is not an error but

 id 
----
  3
(1 row)

which is for certain anomalous, because that is not the row you
saw as being row 2 in the initial FETCH.

The reason for this behavior is that the only-scan-forward check
is in the relatively low-level function PortalRunSelect, which
is passed a "forward" flag and a count.  The place that interprets
FETCH_ABSOLUTE and friends is DoPortalRunFetch, and what it's doing
in this particular scenario is to rewind to start and fetch forwards,
thus bypassing PortalRunSelect's error check.  And, since the query
is using FOR UPDATE, this table scan sees the row with ID=2 as already
dead.  (Its replacement with ID=20 has been installed at the end of
the table, so while it would be visible to the cursor, it's not at
the same position as before.)

So basically, we *do* have this check and have done since 2007,
but it's not water-tight for all variants of FETCH.  I think
tightening it up in HEAD and v14 is a no-brainer, but I'm a bit
more hesitant about whether to back-patch into stable branches.

            regards, tom lane



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Mark Dilger
Дата:
Сообщение: Re: [Patch] ALTER SYSTEM READ ONLY
Следующее
От: Heikki Linnakangas
Дата:
Сообщение: Re: trap instead of error on 32 TiB table