From ba2c6d6cec000f0aeaeda4d56a23a335f6164860 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 8 Jun 2021 17:50:15 -0400 Subject: Avoid misbehavior when persisting a non-stable cursor. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PersistHoldablePortal has long assumed that it should store the entire output of the query-to-be-persisted, which requires rewinding and re-reading the output. This is problematic if the query is not stable: we might get different row contents, or even a different number of rows, which'd confuse the cursor state mightily. In the case where the cursor is NO SCROLL, this is very easy to solve: just store the remaining query output, without any rewinding, and tweak the portal's cursor state to match. Aside from removing the semantic problem, this could be significantly more efficient than storing the whole output. If the cursor is scrollable, there's not much we can do, but it was already the case that scrolling a volatile query's result was pretty unsafe. We can just document more clearly that getting correct results from that is not guaranteed. There are already prohibitions in place on using SCROLL with FOR UPDATE/SHARE, which is one way for a SELECT query to have non-stable results. We could imagine prohibiting SCROLL when the query contains volatile functions, but that would be expensive to enforce. Moreover, it could break applications that work just fine, if they have functions that are in fact stable but the user neglected to mark them so. So settle for documenting the hazard. While this problem has existed in some guise for a long time, it got a lot worse in v11, which introduced the possibility of persisting plpgsql cursors (perhaps implicit ones) even when they violate the rules for what can be marked WITH HOLD. Hence, I've chosen to back-patch to v11 but not further. Per bug #17050 from Алексей Булгаков. Discussion: https://postgr.es/m/17050-f77aa827dc85247c@postgresql.org --- doc/src/sgml/plpgsql.sgml | 9 +++++++++ doc/src/sgml/ref/declare.sgml | 8 +++++--- 2 files changed, 14 insertions(+), 3 deletions(-) (limited to 'doc/src') diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 52f60c827cb..c97344ff927 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -3142,6 +3142,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 aa3d1d1fa16..bbbd335bd0b 100644 --- a/doc/src/sgml/ref/declare.sgml +++ b/doc/src/sgml/ref/declare.sgml @@ -239,12 +239,14 @@ DECLARE name [ BINARY ] [ ASENSITIV - 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. -- cgit v1.2.3