database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-04-27 22:56:53 +03:00
Code

Fix performance bug from conflict between two previous improvements.

Browse Source 
My expanded-objects patch (commit 1dc5ebc9077ab742) included code to make
plpgsql pass expanded-object variables as R/W pointers to certain functions
that are trusted for modifying such variables in-place.  However, that
optimization got broken by commit 6c82d8d1fdb1f126, which arranged to share
a single ParamListInfo across most expressions evaluated by a plpgsql
function.  We don't want a R/W pointer to be passed to other functions
just because we decided one function was safe!  Fortunately, the breakage
was in the other direction, of never passing a R/W pointer at all, because
we'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 unshared
param array to be used when the R/W param optimization is active.

Commit 6c82d8d1fdb1f126 is in HEAD only, so no need for a back-patch.
This commit is contained in:
Tom Lane 2015-08-17 19:39:35 -04:00
parent 47ebbdcee7
commit d3eaab3ef0
1 changed files with 30 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/pl/plpgsql/src/pl_exec.c
View File

@ -5454,12 +5454,18 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
}
/*
* Set up ParamListInfo to pass to executor. For safety, save and restore
* estate->paramLI->parserSetupArg around our use of the param list.
* Set up ParamListInfo to pass to executor. We need an unshared list if
* it's going to include any R/W expanded-object pointer. For safety,
* save and restore estate->paramLI->parserSetupArg around our use of the
* param list.
*/
save_setup_arg = estate->paramLI->parserSetupArg;
paramLI = setup_param_list(estate, expr);
if (expr->rwparam >= 0)
paramLI = setup_unshared_param_list(estate, expr);
else
paramLI = setup_param_list(estate, expr);
econtext->ecxt_param_list_info = paramLI;
/*
@ -5478,6 +5484,9 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
/* Assorted cleanup */
expr->expr_simple_in_use = false;
if (paramLI && paramLI != estate->paramLI)
pfree(paramLI);
estate->paramLI->parserSetupArg = save_setup_arg;
if (!estate->readonly_func)
@ -5504,11 +5513,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
*
* We share a single ParamListInfo array across all SPI calls made from this
* estate, except calls creating cursors, which use setup_unshared_param_list
* (see its comments for reasons why). A shared array is generally OK since
* any given slot in the array would need to contain the same current datum
* value no matter which query or expression we're evaluating. However,
* paramLI->parserSetupArg points to the specific PLpgSQL_expr being
* evaluated. This is not an issue for statement-level callers, but
* (see its comments for reasons why), and calls that pass a R/W expanded
* object pointer. A shared array is generally OK since any given slot in
* the array would need to contain the same current datum value no matter
* which query or expression we're evaluating; but of course that doesn't
* hold when a specific variable is being passed as a R/W pointer, because
* other expressions in the same function probably don't want to do that.
*
* Note that paramLI->parserSetupArg points to the specific PLpgSQL_expr
* being evaluated. This is not an issue for statement-level callers, but
* lower-level callers must save and restore estate->paramLI->parserSetupArg
* just in case there's an active evaluation at an outer call level.
*
@ -5539,6 +5552,11 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
*/
Assert(expr->plan != NULL);
/*
* Expressions with R/W parameters can't use the shared param list.
*/
Assert(expr->rwparam == -1);
/*
* We only need a ParamListInfo if the expression has parameters. In
* 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)
* want it to copy anything that's not used by the specific cursor; that
* could result in uselessly copying some large values.
*
* We also use this for expressions that are passing a R/W object pointer
* to some trusted function. We don't want the R/W pointer to get into the
* shared param list, where it could get passed to some less-trusted function.
*
* Caller should pfree the result after use, if it's not NULL.
*/
static ParamListInfo