Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 1823233

Browse files
committed
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
1 parent c3b5082 commit 1823233

File tree

1 file changed

+14
-7
lines changed

1 file changed

+14
-7
lines changed

src/pl/plpgsql/src/pl_exec.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4540,7 +4540,7 @@ exec_stmt_dynfors(PLpgSQL_execstate *estate, PLpgSQL_stmt_dynfors *stmt)
45404540
int rc;
45414541

45424542
portal = exec_dynquery_with_params(estate, stmt->query, stmt->params,
4543-
NULL, 0);
4543+
NULL, CURSOR_OPT_NO_SCROLL);
45444544

45454545
/*
45464546
* Execute the loop
@@ -5883,14 +5883,21 @@ exec_run_select(PLpgSQL_execstate *estate,
58835883
* On the first call for this expression generate the plan.
58845884
*
58855885
* If we don't need to return a portal, then we're just going to execute
5886-
* the query once, which means it's OK to use a parallel plan, even if the
5887-
* number of rows being fetched is limited. If we do need to return a
5888-
* portal, the caller might do cursor operations, which parallel query
5889-
* can't support.
5886+
* the query immediately, which means it's OK to use a parallel plan, even
5887+
* if the number of rows being fetched is limited. If we do need to
5888+
* return a portal (i.e., this is for a FOR loop), the user's code might
5889+
* invoke additional operations inside the FOR loop, making parallel query
5890+
* unsafe. In any case, we don't expect any cursor operations to be done,
5891+
* so specify NO_SCROLL for efficiency and semantic safety.
58905892
*/
58915893
if (expr->plan == NULL)
5892-
exec_prepare_plan(estate, expr,
5893-
portalP == NULL ? CURSOR_OPT_PARALLEL_OK : 0, true);
5894+
{
5895+
int cursorOptions = CURSOR_OPT_NO_SCROLL;
5896+
5897+
if (portalP == NULL)
5898+
cursorOptions |= CURSOR_OPT_PARALLEL_OK;
5899+
exec_prepare_plan(estate, expr, cursorOptions, true);
5900+
}
58945901

58955902
/*
58965903
* Set up ParamListInfo to pass to executor

0 commit comments

Comments
 (0)