Re: We don't enforce NO SCROLL cursor restrictions
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Vik Fearing <vik@postgresfriends.org>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2021-09-09T19:21:20Z
Lists: 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
Commits
-
Fix some anomalies with NO SCROLL cursors.
- fa5d0415f10f 13.5 landed
- d844cd75a676 14.0 landed
- c1b7a6c27312 15.0 landed
- ba408fc960b6 12.9 landed
- 9ea8d3d24a9f 11.14 landed
-
Avoid misbehavior when persisting a non-stable cursor.
- ba2c6d6cec00 14.0 cited