Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc1b7a6c

Browse files
committed
Fix some anomalies with NO SCROLL cursors.
We have long forbidden fetching backwards from a NO SCROLL cursor,but the prohibition didn't extend to cases in which we rewind thequery altogether and then re-fetch forwards. I think the reason isthat this logic was mainly meant to protect plan nodes that can'tbe run in the reverse direction. However, re-reading the query outputis problematic if the query is volatile (which includes SELECT FORUPDATE, not just queries with volatile functions): the re-read canproduce different results, which confuses the cursor navigation logiccompletely. Another reason for disliking this approach is that somecode paths will either fetch backwards or rewind-and-fetch-forwardsdepending on the distance to the target row; so that seeminglyidentical use-cases may or may not draw the "cursor can only scanforward" error. Hence, let's clean things up by disallowing rewindas well as fetch-backwards in a NO SCROLL cursor.Ordinarily we'd only make such a definitional change in HEAD, butthere is a third reason to consider this change now. Commitba2c6d6created some new user-visible anomalies for non-scrollable cursorsWITH HOLD, in that navigation in the cursor result got confused if thecursor had been partially read before committing. The only good wayto resolve those anomalies is to forbid rewinding such a cursor, whichallows removal of the incorrect cursor state manipulations thatba2c6d6 added to PersistHoldablePortal.To minimize the behavioral change in the back branches (includingv14), refuse to rewind a NO SCROLL cursor only when it has a holdStore,ie has been held over from a previous transaction due to WITH HOLD.This should avoid breaking most applications that have been sloppyabout whether to declare cursors as scrollable. We'll enforce theprohibition across-the-board beginning in v15.Back-patch to v11, asba2c6d6 was.Discussion:https://postgr.es/m/3712911.1631207435@sss.pgh.pa.us
1 parent2d689f2 commitc1b7a6c

File tree

6 files changed

+134
-13
lines changed

6 files changed

+134
-13
lines changed

‎src/backend/commands/portalcmds.c

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -375,17 +375,22 @@ PersistHoldablePortal(Portal portal)
375375
* can be processed. Otherwise, store only the not-yet-fetched rows.
376376
* (The latter is not only more efficient, but avoids semantic
377377
* problems if the query's output isn't stable.)
378+
*
379+
* In the no-scroll case, tuple indexes in the tuplestore will not
380+
* match the cursor's nominal position (portalPos). Currently this
381+
* causes no difficulty because we only navigate in the tuplestore by
382+
* relative position, except for the tuplestore_skiptuples call below
383+
* and the tuplestore_rescan call in DoPortalRewind, both of which are
384+
* disabled for no-scroll cursors. But someday we might need to track
385+
* the offset between the holdStore and the cursor's nominal position
386+
* explicitly.
378387
*/
379388
if (portal->cursorOptions&CURSOR_OPT_SCROLL)
380389
{
381390
ExecutorRewind(queryDesc);
382391
}
383392
else
384393
{
385-
/* Disallow moving backwards from here */
386-
portal->atStart= true;
387-
portal->portalPos=0;
388-
389394
/*
390395
* If we already reached end-of-query, set the direction to
391396
* NoMovement to avoid trying to fetch any tuples. (This check
@@ -443,10 +448,17 @@ PersistHoldablePortal(Portal portal)
443448
{
444449
tuplestore_rescan(portal->holdStore);
445450

446-
if (!tuplestore_skiptuples(portal->holdStore,
447-
portal->portalPos,
448-
true))
449-
elog(ERROR,"unexpected end of tuple stream");
451+
/*
452+
* In the no-scroll case, the start of the tuplestore is exactly
453+
* where we want to be, so no repositioning is wanted.
454+
*/
455+
if (portal->cursorOptions&CURSOR_OPT_SCROLL)
456+
{
457+
if (!tuplestore_skiptuples(portal->holdStore,
458+
portal->portalPos,
459+
true))
460+
elog(ERROR,"unexpected end of tuple stream");
461+
}
450462
}
451463
}
452464
PG_CATCH();

‎src/backend/tcop/pquery.c

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,9 +1472,8 @@ PortalRunFetch(Portal portal,
14721472
* DoPortalRunFetch
14731473
*Guts of PortalRunFetch --- the portal context is already set up
14741474
*
1475-
* count <= 0 is interpreted as a no-op: the destination gets started up
1476-
* and shut down, but nothing else happens. Also, count == FETCH_ALL is
1477-
* interpreted as "all rows". (cf FetchStmt.howMany)
1475+
* Here, count < 0 typically reverses the direction. Also, count == FETCH_ALL
1476+
* is interpreted as "all rows". (cf FetchStmt.howMany)
14781477
*
14791478
* Returns number of rows processed (suitable for use in result tag)
14801479
*/
@@ -1491,6 +1490,15 @@ DoPortalRunFetch(Portal portal,
14911490
portal->strategy==PORTAL_ONE_MOD_WITH||
14921491
portal->strategy==PORTAL_UTIL_SELECT);
14931492

1493+
/*
1494+
* Note: we disallow backwards fetch (including re-fetch of current row)
1495+
* for NO SCROLL cursors, but we interpret that very loosely: you can use
1496+
* any of the FetchDirection options, so long as the end result is to move
1497+
* forwards by at least one row. Currently it's sufficient to check for
1498+
* NO SCROLL in DoPortalRewind() and in the forward == false path in
1499+
* PortalRunSelect(); but someday we might prefer to account for that
1500+
* restriction explicitly here.
1501+
*/
14941502
switch (fdirection)
14951503
{
14961504
caseFETCH_FORWARD:
@@ -1668,6 +1676,20 @@ DoPortalRewind(Portal portal)
16681676
{
16691677
QueryDesc*queryDesc;
16701678

1679+
/*
1680+
* No work is needed if we've not advanced nor attempted to advance the
1681+
* cursor (and we don't want to throw a NO SCROLL error in this case).
1682+
*/
1683+
if (portal->atStart&& !portal->atEnd)
1684+
return;
1685+
1686+
/* Otherwise, cursor must allow scrolling */
1687+
if (portal->cursorOptions&CURSOR_OPT_NO_SCROLL)
1688+
ereport(ERROR,
1689+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
1690+
errmsg("cursor can only scan forward"),
1691+
errhint("Declare it with SCROLL option to enable backward scan.")));
1692+
16711693
/* Rewind holdStore, if we have one */
16721694
if (portal->holdStore)
16731695
{

‎src/test/regress/expected/portals.out

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,24 @@ FETCH BACKWARD 1 FROM foo24; -- should fail
715715
ERROR: cursor can only scan forward
716716
HINT: Declare it with SCROLL option to enable backward scan.
717717
END;
718+
BEGIN;
719+
DECLARE foo24 NO SCROLL CURSOR FOR SELECT * FROM tenk1 ORDER BY unique2;
720+
FETCH 1 FROM foo24;
721+
unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4
722+
---------+---------+-----+------+-----+--------+---------+----------+-------------+-----------+----------+-----+------+----------+----------+---------
723+
8800 | 0 | 0 | 0 | 0 | 0 | 0 | 800 | 800 | 3800 | 8800 | 0 | 1 | MAAAAA | AAAAAA | AAAAxx
724+
(1 row)
725+
726+
FETCH ABSOLUTE 2 FROM foo24; -- allowed
727+
unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4
728+
---------+---------+-----+------+-----+--------+---------+----------+-------------+-----------+----------+-----+------+----------+----------+---------
729+
1891 | 1 | 1 | 3 | 1 | 11 | 91 | 891 | 1891 | 1891 | 1891 | 182 | 183 | TUAAAA | BAAAAA | HHHHxx
730+
(1 row)
731+
732+
FETCH ABSOLUTE 1 FROM foo24; -- should fail
733+
ERROR: cursor can only scan forward
734+
HINT: Declare it with SCROLL option to enable backward scan.
735+
END;
718736
--
719737
-- Cursors outside transaction blocks
720738
--
@@ -763,6 +781,43 @@ SELECT name, statement, is_holdable, is_binary, is_scrollable FROM pg_cursors;
763781
(1 row)
764782

765783
CLOSE foo25;
784+
BEGIN;
785+
DECLARE foo25ns NO SCROLL CURSOR WITH HOLD FOR SELECT * FROM tenk2;
786+
FETCH FROM foo25ns;
787+
unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4
788+
---------+---------+-----+------+-----+--------+---------+----------+-------------+-----------+----------+-----+------+----------+----------+---------
789+
8800 | 0 | 0 | 0 | 0 | 0 | 0 | 800 | 800 | 3800 | 8800 | 0 | 1 | MAAAAA | AAAAAA | AAAAxx
790+
(1 row)
791+
792+
FETCH FROM foo25ns;
793+
unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4
794+
---------+---------+-----+------+-----+--------+---------+----------+-------------+-----------+----------+-----+------+----------+----------+---------
795+
1891 | 1 | 1 | 3 | 1 | 11 | 91 | 891 | 1891 | 1891 | 1891 | 182 | 183 | TUAAAA | BAAAAA | HHHHxx
796+
(1 row)
797+
798+
COMMIT;
799+
FETCH FROM foo25ns;
800+
unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4
801+
---------+---------+-----+------+-----+--------+---------+----------+-------------+-----------+----------+-----+------+----------+----------+---------
802+
3420 | 2 | 0 | 0 | 0 | 0 | 20 | 420 | 1420 | 3420 | 3420 | 40 | 41 | OBAAAA | CAAAAA | OOOOxx
803+
(1 row)
804+
805+
FETCH ABSOLUTE 4 FROM foo25ns;
806+
unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4
807+
---------+---------+-----+------+-----+--------+---------+----------+-------------+-----------+----------+-----+------+----------+----------+---------
808+
9850 | 3 | 0 | 2 | 0 | 10 | 50 | 850 | 1850 | 4850 | 9850 | 100 | 101 | WOAAAA | DAAAAA | VVVVxx
809+
(1 row)
810+
811+
FETCH ABSOLUTE 4 FROM foo25ns; -- fail
812+
ERROR: cursor can only scan forward
813+
HINT: Declare it with SCROLL option to enable backward scan.
814+
SELECT name, statement, is_holdable, is_binary, is_scrollable FROM pg_cursors;
815+
name | statement | is_holdable | is_binary | is_scrollable
816+
---------+---------------------------------------------------------------------+-------------+-----------+---------------
817+
foo25ns | DECLARE foo25ns NO SCROLL CURSOR WITH HOLD FOR SELECT * FROM tenk2; | t | f | f
818+
(1 row)
819+
820+
CLOSE foo25ns;
766821
--
767822
-- ROLLBACK should close holdable cursors
768823
--

‎src/test/regress/expected/tablesample.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ View definition:
8888

8989
-- check a sampled query doesn't affect cursor in progress
9090
BEGIN;
91-
DECLARE tablesample_cur CURSOR FOR
91+
DECLARE tablesample_curSCROLLCURSOR FOR
9292
SELECT id FROM test_tablesample TABLESAMPLE SYSTEM (50) REPEATABLE (0);
9393
FETCH FIRST FROM tablesample_cur;
9494
id

‎src/test/regress/sql/portals.sql

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,18 @@ FETCH BACKWARD 1 FROM foo24; -- should fail
190190

191191
END;
192192

193+
BEGIN;
194+
195+
DECLARE foo24 NO SCROLL CURSOR FORSELECT*FROM tenk1ORDER BY unique2;
196+
197+
FETCH1FROM foo24;
198+
199+
FETCH ABSOLUTE2FROM foo24;-- allowed
200+
201+
FETCH ABSOLUTE1FROM foo24;-- should fail
202+
203+
END;
204+
193205
--
194206
-- Cursors outside transaction blocks
195207
--
@@ -217,6 +229,26 @@ SELECT name, statement, is_holdable, is_binary, is_scrollable FROM pg_cursors;
217229

218230
CLOSE foo25;
219231

232+
BEGIN;
233+
234+
DECLARE foo25ns NO SCROLL CURSOR WITH HOLD FORSELECT*FROM tenk2;
235+
236+
FETCHFROM foo25ns;
237+
238+
FETCHFROM foo25ns;
239+
240+
COMMIT;
241+
242+
FETCHFROM foo25ns;
243+
244+
FETCH ABSOLUTE4FROM foo25ns;
245+
246+
FETCH ABSOLUTE4FROM foo25ns;-- fail
247+
248+
SELECT name, statement, is_holdable, is_binary, is_scrollableFROM pg_cursors;
249+
250+
CLOSE foo25ns;
251+
220252
--
221253
-- ROLLBACK should close holdable cursors
222254
--

‎src/test/regress/sql/tablesample.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ CREATE VIEW test_tablesample_v2 AS
2424

2525
-- check a sampled query doesn't affect cursor in progress
2626
BEGIN;
27-
DECLARE tablesample_cur CURSOR FOR
27+
DECLARE tablesample_curSCROLLCURSOR FOR
2828
SELECT idFROM test_tablesample TABLESAMPLE SYSTEM (50) REPEATABLE (0);
2929

3030
FETCH FIRSTFROM tablesample_cur;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp