diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index b562bfd9c30..5c546f630fe 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3033,6 +3033,15 @@ DECLARE
is said to be unbound since it is not bound to
any particular query.
+
+
+ The SCROLL option cannot be used when the cursor's
+ query uses FOR UPDATE/SHARE. Also, it is
+ best to use NO SCROLL with a query that involves
+ volatile functions. The implementation of SCROLL
+ assumes that re-reading the query's output will give consistent
+ results, which a volatile function might not do.
+
diff --git a/doc/src/sgml/ref/declare.sgml b/doc/src/sgml/ref/declare.sgml
index d6177dcd9c4..951dfa982ba 100644
--- a/doc/src/sgml/ref/declare.sgml
+++ b/doc/src/sgml/ref/declare.sgml
@@ -228,12 +228,14 @@ DECLARE name [ BINARY ] [ INSENSITI
- Scrollable and WITH HOLD cursors may give unexpected
+ Scrollable cursors may give unexpected
results if they invoke any volatile functions (see ). When a previously fetched row is
re-fetched, the functions might be re-executed, perhaps leading to
- results different from the first time. One workaround for such cases
- is to declare the cursor WITH HOLD and commit the
+ results different from the first time. It's best to
+ specify NO SCROLL for a query involving volatile
+ functions. If that is not practical, one workaround
+ is to declare the cursor SCROLL WITH HOLD and commit the
transaction before reading any rows from it. This will force the
entire output of the cursor to be materialized in temporary storage,
so that volatile functions are executed exactly once for each row.
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index f65529ba6ad..4bd7ec1ba44 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -374,10 +374,23 @@ PersistHoldablePortal(Portal portal)
PushActiveSnapshot(queryDesc->snapshot);
/*
- * Rewind the executor: we need to store the entire result set in the
- * tuplestore, so that subsequent backward FETCHs can be processed.
+ * If the portal is marked scrollable, we need to store the entire
+ * result set in the tuplestore, so that subsequent backward FETCHs
+ * can be processed. Otherwise, store only the not-yet-fetched rows.
+ * (The latter is not only more efficient, but avoids semantic
+ * problems if the query's output isn't stable.)
*/
- ExecutorRewind(queryDesc);
+ if (portal->cursorOptions & CURSOR_OPT_SCROLL)
+ {
+ ExecutorRewind(queryDesc);
+ }
+ else
+ {
+ /* We must reset the cursor state as though at start of query */
+ portal->atStart = true;
+ portal->atEnd = false;
+ portal->portalPos = 0;
+ }
/*
* Change the destination to output to the tuplestore. Note we tell
diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
index e205a1e0022..918cc0913e6 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
@@ -335,6 +335,57 @@ SELECT * FROM pg_cursors;
------+-----------+-------------+-----------+---------------+---------------
(0 rows)
+-- interaction of FOR UPDATE cursor with subsequent updates (bug #17050)
+TRUNCATE test1;
+INSERT INTO test1 VALUES (1,'one'), (2,'two'), (3,'three');
+DO LANGUAGE plpgsql $$
+DECLARE
+ l_cur CURSOR FOR SELECT a FROM test1 ORDER BY 1 FOR UPDATE;
+BEGIN
+ FOR r IN l_cur LOOP
+ UPDATE test1 SET b = b || ' ' || b WHERE a = r.a;
+ COMMIT;
+ END LOOP;
+END;
+$$;
+SELECT * FROM test1;
+ a | b
+---+-------------
+ 1 | one one
+ 2 | two two
+ 3 | three three
+(3 rows)
+
+SELECT * FROM pg_cursors;
+ name | statement | is_holdable | is_binary | is_scrollable | creation_time
+------+-----------+-------------+-----------+---------------+---------------
+(0 rows)
+
+-- like bug #17050, but with implicit cursor
+TRUNCATE test1;
+INSERT INTO test1 VALUES (1,'one'), (2,'two'), (3,'three');
+DO LANGUAGE plpgsql $$
+DECLARE r RECORD;
+BEGIN
+ FOR r IN SELECT a FROM test1 FOR UPDATE LOOP
+ UPDATE test1 SET b = b || ' ' || b WHERE a = r.a;
+ COMMIT;
+ END LOOP;
+END;
+$$;
+SELECT * FROM test1;
+ a | b
+---+-------------
+ 1 | one one
+ 2 | two two
+ 3 | three three
+(3 rows)
+
+SELECT * FROM pg_cursors;
+ name | statement | is_holdable | is_binary | is_scrollable | creation_time
+------+-----------+-------------+-----------+---------------+---------------
+(0 rows)
+
-- commit inside block with exception handler
TRUNCATE test1;
DO LANGUAGE plpgsql $$
diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
index 94fd406b7a3..cc26788b9ae 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
@@ -273,6 +273,47 @@ SELECT * FROM test2;
SELECT * FROM pg_cursors;
+-- interaction of FOR UPDATE cursor with subsequent updates (bug #17050)
+TRUNCATE test1;
+
+INSERT INTO test1 VALUES (1,'one'), (2,'two'), (3,'three');
+
+DO LANGUAGE plpgsql $$
+DECLARE
+ l_cur CURSOR FOR SELECT a FROM test1 ORDER BY 1 FOR UPDATE;
+BEGIN
+ FOR r IN l_cur LOOP
+ UPDATE test1 SET b = b || ' ' || b WHERE a = r.a;
+ COMMIT;
+ END LOOP;
+END;
+$$;
+
+SELECT * FROM test1;
+
+SELECT * FROM pg_cursors;
+
+
+-- like bug #17050, but with implicit cursor
+TRUNCATE test1;
+
+INSERT INTO test1 VALUES (1,'one'), (2,'two'), (3,'three');
+
+DO LANGUAGE plpgsql $$
+DECLARE r RECORD;
+BEGIN
+ FOR r IN SELECT a FROM test1 FOR UPDATE LOOP
+ UPDATE test1 SET b = b || ' ' || b WHERE a = r.a;
+ COMMIT;
+ END LOOP;
+END;
+$$;
+
+SELECT * FROM test1;
+
+SELECT * FROM pg_cursors;
+
+
-- commit inside block with exception handler
TRUNCATE test1;