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

Commitd3eaab3

Browse files
committed
Fix performance bug from conflict between two previous improvements.
My expanded-objects patch (commit1dc5ebc) included code to makeplpgsql pass expanded-object variables as R/W pointers to certain functionsthat are trusted for modifying such variables in-place. However, thatoptimization got broken by commit6c82d8d, which arranged to sharea single ParamListInfo across most expressions evaluated by a plpgsqlfunction. We don't want a R/W pointer to be passed to other functionsjust because we decided one function was safe! Fortunately, the breakagewas in the other direction, of never passing a R/W pointer at all, becausewe'd always have pre-initialized the shared array slot with a R/O pointer.So it was still functionally correct, but we were back to O(N^2)performance for repeated use of "a := a || x". To fix, force an unsharedparam array to be used when the R/W param optimization is active.Commit6c82d8d is in HEAD only, so no need for a back-patch.
1 parent47ebbdc commitd3eaab3

File tree

1 file changed

+30
-8
lines changed

1 file changed

+30
-8
lines changed

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5454,12 +5454,18 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
54545454
}
54555455

54565456
/*
5457-
* Set up ParamListInfo to pass to executor. For safety, save and restore
5458-
* estate->paramLI->parserSetupArg around our use of the param list.
5457+
* Set up ParamListInfo to pass to executor. We need an unshared list if
5458+
* it's going to include any R/W expanded-object pointer. For safety,
5459+
* save and restore estate->paramLI->parserSetupArg around our use of the
5460+
* param list.
54595461
*/
54605462
save_setup_arg=estate->paramLI->parserSetupArg;
54615463

5462-
paramLI=setup_param_list(estate,expr);
5464+
if (expr->rwparam >=0)
5465+
paramLI=setup_unshared_param_list(estate,expr);
5466+
else
5467+
paramLI=setup_param_list(estate,expr);
5468+
54635469
econtext->ecxt_param_list_info=paramLI;
54645470

54655471
/*
@@ -5478,6 +5484,9 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
54785484
/* Assorted cleanup */
54795485
expr->expr_simple_in_use= false;
54805486

5487+
if (paramLI&&paramLI!=estate->paramLI)
5488+
pfree(paramLI);
5489+
54815490
estate->paramLI->parserSetupArg=save_setup_arg;
54825491

54835492
if (!estate->readonly_func)
@@ -5504,11 +5513,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
55045513
*
55055514
* We share a single ParamListInfo array across all SPI calls made from this
55065515
* estate, except calls creating cursors, which use setup_unshared_param_list
5507-
* (see its comments for reasons why). A shared array is generally OK since
5508-
* any given slot in the array would need to contain the same current datum
5509-
* value no matter which query or expression we're evaluating. However,
5510-
* paramLI->parserSetupArg points to the specific PLpgSQL_expr being
5511-
* evaluated. This is not an issue for statement-level callers, but
5516+
* (see its comments for reasons why), and calls that pass a R/W expanded
5517+
* object pointer. A shared array is generally OK since any given slot in
5518+
* the array would need to contain the same current datum value no matter
5519+
* which query or expression we're evaluating; but of course that doesn't
5520+
* hold when a specific variable is being passed as a R/W pointer, because
5521+
* other expressions in the same function probably don't want to do that.
5522+
*
5523+
* Note that paramLI->parserSetupArg points to the specific PLpgSQL_expr
5524+
* being evaluated. This is not an issue for statement-level callers, but
55125525
* lower-level callers must save and restore estate->paramLI->parserSetupArg
55135526
* just in case there's an active evaluation at an outer call level.
55145527
*
@@ -5539,6 +5552,11 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
55395552
*/
55405553
Assert(expr->plan!=NULL);
55415554

5555+
/*
5556+
* Expressions with R/W parameters can't use the shared param list.
5557+
*/
5558+
Assert(expr->rwparam==-1);
5559+
55425560
/*
55435561
* We only need a ParamListInfo if the expression has parameters. In
55445562
* principle we should test with bms_is_empty(), but we use a not-null
@@ -5605,6 +5623,10 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
56055623
* want it to copy anything that's not used by the specific cursor; that
56065624
* could result in uselessly copying some large values.
56075625
*
5626+
* We also use this for expressions that are passing a R/W object pointer
5627+
* to some trusted function. We don't want the R/W pointer to get into the
5628+
* shared param list, where it could get passed to some less-trusted function.
5629+
*
56085630
* Caller should pfree the result after use, if it's not NULL.
56095631
*/
56105632
staticParamListInfo

0 commit comments

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