Fix performance bug from conflict between two previous improvements.

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.

1 files changed, 30 insertions, 8 deletions

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 8bc5e6f29fd..fb9333606d4 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -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.
  *
@@ -5540,6 +5553,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
 	 * test because it's faster.  In current usage bits are never removed from
@@ -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