From: Tom Lane <tgl@sss.pgh.pa.us>
To: Vik Fearing <vik@postgresfriends.org>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: We don't enforce NO SCROLL cursor restrictions
Date: Thu, 09 Sep 2021 15:21:20 -0400
Message-ID: <3725815.1631215280@sss.pgh.pa.us> (raw)
In-Reply-To: <3722175.1631210998@sss.pgh.pa.us>
References: <3712911.1631207435@sss.pgh.pa.us>
<e6b5e1c3-71ad-b384-7f6f-0c3f021906d1@postgresfriends.org>
<3722175.1631210998@sss.pgh.pa.us>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, vik@postgresfriends.org, pgsql-hackers@lists.postgresql.org
Subject: Re: We don't enforce NO SCROLL cursor restrictions
In-Reply-To: <3725815.1631215280@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox