Force NO SCROLL for plpgsql's implicit cursors.

 Further thought about bug #17050 suggests that it's a good idea
 to use CURSOR_OPT_NO_SCROLL for the implicit cursor opened by
 a plpgsql FOR-over-query loop.  This ensures that, if somebody
 commits inside the loop, PersistHoldablePortal won't try to
 rewind and re-read the cursor.  While we'd have selected NO_SCROLL
 anyway if FOR UPDATE/SHARE appears in the query, there are other
 hazards with volatile functions; and in any case, it's silly to
 expend effort storing rows that we know for certain won't be needed.
 
 (While here, improve the comment in exec_run_select, which was a bit
 confused about the rationale for when we can use parallel mode.
 Cursor operations aren't a hazard for nameless portals.)
 
 This wasn't an issue until v11, which introduced the possibility
 of persisting such cursors.  Hence, back-patch to v11.
 
 Per bug #17050 from Алексей Булгаков.
 
 Discussion: https://postgr.es/m/17050-f77aa827dc85247c@postgresql.org
 
 

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 78b593d12c7ce2862fc2744f171439f06667bcdb..207c4424facbfeebf25b135aad04f96cb1142aae 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -4561,7 +4561,7 @@ exec_stmt_dynfors(PLpgSQL_execstate *estate, PLpgSQL_stmt_dynfors *stmt)
    int         rc;
 
    portal = exec_dynquery_with_params(estate, stmt->query, stmt->params,
-                                      NULL, 0);
+                                      NULL, CURSOR_OPT_NO_SCROLL);
 
    /*
     * Execute the loop
@@ -5694,14 +5694,21 @@ exec_run_select(PLpgSQL_execstate *estate,
     * On the first call for this expression generate the plan.
     *
     * If we don't need to return a portal, then we're just going to execute
-    * the query once, which means it's OK to use a parallel plan, even if the
-    * number of rows being fetched is limited.  If we do need to return a
-    * portal, the caller might do cursor operations, which parallel query
-    * can't support.
+    * the query immediately, which means it's OK to use a parallel plan, even
+    * if the number of rows being fetched is limited.  If we do need to
+    * return a portal (i.e., this is for a FOR loop), the user's code might
+    * invoke additional operations inside the FOR loop, making parallel query
+    * unsafe.  In any case, we don't expect any cursor operations to be done,
+    * so specify NO_SCROLL for efficiency and semantic safety.
     */
    if (expr->plan == NULL)
-       exec_prepare_plan(estate, expr,
-                         portalP == NULL ? CURSOR_OPT_PARALLEL_OK : 0);
+   {
+       int         cursorOptions = CURSOR_OPT_NO_SCROLL;
+
+       if (portalP == NULL)
+           cursorOptions |= CURSOR_OPT_PARALLEL_OK;
+       exec_prepare_plan(estate, expr, cursorOptions);
+   }
 
    /*
     * Set up ParamListInfo to pass to executor