Replace uses of SPI_modifytuple that intend to allocate in current context.

Invent a new function heap_modify_tuple_by_cols() that is functionally
equivalent to SPI_modifytuple except that it always allocates its result
by simple palloc.  I chose however to make the API details a bit more
like heap_modify_tuple: pass a tupdesc rather than a Relation, and use
bool convention for the isnull array.

Use this function in place of SPI_modifytuple at all call sites where the
intended behavior is to allocate in current context.  (There actually are
only two call sites left that depend on the old behavior, which makes me
wonder if we should just drop this function rather than keep it.)

This new function is easier to use than heap_modify_tuple() for purposes
of replacing a single column (or, really, any fixed number of columns).
There are a number of places where it would simplify the code to change
over, but I resisted that temptation for the moment ... everywhere except
in plpgsql's exec_assign_value(); changing that might offer some small
performance benefit, so I did it.

This is on the way to removing SPI_push/SPI_pop, but it seems like
good code cleanup in its own right.

Discussion: <9633.1478552022@sss.pgh.pa.us>

5 files changed, 101 insertions, 55 deletions

diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 6d0f3f37673..e27ec78b714 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -847,6 +847,72 @@ heap_modify_tuple(HeapTuple tuple,
 }
 
 /*
+ * heap_modify_tuple_by_cols
+ *		form a new tuple from an old tuple and a set of replacement values.
+ *
+ * This is like heap_modify_tuple, except that instead of specifying which
+ * column(s) to replace by a boolean map, an array of target column numbers
+ * is used.  This is often more convenient when a fixed number of columns
+ * are to be replaced.  The replCols, replValues, and replIsnull arrays must
+ * be of length nCols.  Target column numbers are indexed from 1.
+ *
+ * The result is allocated in the current memory context.
+ */
+HeapTuple
+heap_modify_tuple_by_cols(HeapTuple tuple,
+						  TupleDesc tupleDesc,
+						  int nCols,
+						  int *replCols,
+						  Datum *replValues,
+						  bool *replIsnull)
+{
+	int			numberOfAttributes = tupleDesc->natts;
+	Datum	   *values;
+	bool	   *isnull;
+	HeapTuple	newTuple;
+	int			i;
+
+	/*
+	 * allocate and fill values and isnull arrays from the tuple, then replace
+	 * selected columns from the input arrays.
+	 */
+	values = (Datum *) palloc(numberOfAttributes * sizeof(Datum));
+	isnull = (bool *) palloc(numberOfAttributes * sizeof(bool));
+
+	heap_deform_tuple(tuple, tupleDesc, values, isnull);
+
+	for (i = 0; i < nCols; i++)
+	{
+		int			attnum = replCols[i];
+
+		if (attnum <= 0 || attnum > numberOfAttributes)
+			elog(ERROR, "invalid column number %d", attnum);
+		values[attnum - 1] = replValues[i];
+		isnull[attnum - 1] = replIsnull[i];
+	}
+
+	/*
+	 * create a new tuple from the values and isnull arrays
+	 */
+	newTuple = heap_form_tuple(tupleDesc, values, isnull);
+
+	pfree(values);
+	pfree(isnull);
+
+	/*
+	 * copy the identification info of the old tuple: t_ctid, t_self, and OID
+	 * (if any)
+	 */
+	newTuple->t_data->t_ctid = tuple->t_data->t_ctid;
+	newTuple->t_self = tuple->t_self;
+	newTuple->t_tableOid = tuple->t_tableOid;
+	if (tupleDesc->tdhasoid)
+		HeapTupleSetOid(newTuple, HeapTupleGetOid(tuple));
+
+	return newTuple;
+}
+
+/*
  * heap_deform_tuple
  *		Given a tuple, extract data into values/isnull arrays; this is
  *		the inverse of heap_form_tuple.
diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index 0e9ae5ff9cf..c9d5060f2c7 100644
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -2329,8 +2329,10 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
 	if (prs.curwords)
 	{
 		datum = PointerGetDatum(make_tsvector(&prs));
-		rettuple = SPI_modifytuple(rel, rettuple, 1, &tsvector_attr_num,
-								   &datum, NULL);
+		isnull = false;
+		rettuple = heap_modify_tuple_by_cols(rettuple, rel->rd_att,
+											 1, &tsvector_attr_num,
+											 &datum, &isnull);
 		pfree(DatumGetPointer(datum));
 	}
 	else
@@ -2340,14 +2342,12 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
 		SET_VARSIZE(out, CALCDATASIZE(0, 0));
 		out->size = 0;
 		datum = PointerGetDatum(out);
-		rettuple = SPI_modifytuple(rel, rettuple, 1, &tsvector_attr_num,
-								   &datum, NULL);
+		isnull = false;
+		rettuple = heap_modify_tuple_by_cols(rettuple, rel->rd_att,
+											 1, &tsvector_attr_num,
+											 &datum, &isnull);
 		pfree(prs.words);
 	}
 
-	if (rettuple == NULL)		/* internal error */
-		elog(ERROR, "tsvector_update_trigger: %d returned by SPI_modifytuple",
-			 SPI_result);
-
 	return PointerGetDatum(rettuple);
 }
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index d7e5fad11e5..8fb1f6ddea1 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -805,6 +805,12 @@ extern HeapTuple heap_modify_tuple(HeapTuple tuple,
 				  Datum *replValues,
 				  bool *replIsnull,
 				  bool *doReplace);
+extern HeapTuple heap_modify_tuple_by_cols(HeapTuple tuple,
+						  TupleDesc tupleDesc,
+						  int nCols,
+						  int *replCols,
+						  Datum *replValues,
+						  bool *replIsnull);
 extern void heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc,
 				  Datum *values, bool *isnull);
 extern void heap_freetuple(HeapTuple htup);
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 042b31fd77f..91e1f8dd3fd 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -4562,10 +4562,9 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				PLpgSQL_rec *rec;
 				int			fno;
 				HeapTuple	newtup;
-				int			natts;
-				Datum	   *values;
-				bool	   *nulls;
-				bool	   *replaces;
+				int			colnums[1];
+				Datum		values[1];
+				bool		nulls[1];
 				Oid			atttype;
 				int32		atttypmod;
 
@@ -4584,9 +4583,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
 						   errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
 
 				/*
-				 * Get the number of the records field to change and the
-				 * number of attributes in the tuple.  Note: disallow system
-				 * column names because the code below won't cope.
+				 * Get the number of the record field to change.  Disallow
+				 * system columns because the code below won't cope.
 				 */
 				fno = SPI_fnumber(rec->tupdesc, recfield->fieldname);
 				if (fno <= 0)
@@ -4594,42 +4592,25 @@ exec_assign_value(PLpgSQL_execstate *estate,
 							(errcode(ERRCODE_UNDEFINED_COLUMN),
 							 errmsg("record \"%s\" has no field \"%s\"",
 									rec->refname, recfield->fieldname)));
-				fno--;
-				natts = rec->tupdesc->natts;
-
-				/*
-				 * Set up values/control arrays for heap_modify_tuple. For all
-				 * the attributes except the one we want to replace, use the
-				 * value that's in the old tuple.
-				 */
-				values = eval_mcontext_alloc(estate, sizeof(Datum) * natts);
-				nulls = eval_mcontext_alloc(estate, sizeof(bool) * natts);
-				replaces = eval_mcontext_alloc(estate, sizeof(bool) * natts);
-
-				memset(replaces, false, sizeof(bool) * natts);
-				replaces[fno] = true;
+				colnums[0] = fno;
 
 				/*
 				 * Now insert the new value, being careful to cast it to the
 				 * right type.
 				 */
-				atttype = rec->tupdesc->attrs[fno]->atttypid;
-				atttypmod = rec->tupdesc->attrs[fno]->atttypmod;
-				values[fno] = exec_cast_value(estate,
-											  value,
-											  &isNull,
-											  valtype,
-											  valtypmod,
-											  atttype,
-											  atttypmod);
-				nulls[fno] = isNull;
-
-				/*
-				 * Now call heap_modify_tuple() to create a new tuple that
-				 * replaces the old one in the record.
-				 */
-				newtup = heap_modify_tuple(rec->tup, rec->tupdesc,
-										   values, nulls, replaces);
+				atttype = rec->tupdesc->attrs[fno - 1]->atttypid;
+				atttypmod = rec->tupdesc->attrs[fno - 1]->atttypmod;
+				values[0] = exec_cast_value(estate,
+											value,
+											&isNull,
+											valtype,
+											valtypmod,
+											atttype,
+											atttypmod);
+				nulls[0] = isNull;
+
+				newtup = heap_modify_tuple_by_cols(rec->tup, rec->tupdesc,
+												   1, colnums, values, nulls);
 
 				if (rec->freetup)
 					heap_freetuple(rec->tup);
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 119a59ab073..32703fcdcf9 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -639,15 +639,8 @@ ttdummy(PG_FUNCTION_ARGS)
 
 	/* Tuple to return to upper Executor ... */
 	if (newtuple)				/* UPDATE */
-	{
-		HeapTuple	tmptuple;
-
-		tmptuple = SPI_copytuple(trigtuple);
-		rettuple = SPI_modifytuple(rel, tmptuple, 1, &(attnum[1]), &newoff, NULL);
-		SPI_freetuple(tmptuple);
-	}
-	else
-		/* DELETE */
+		rettuple = SPI_modifytuple(rel, trigtuple, 1, &(attnum[1]), &newoff, NULL);
+	else	/* DELETE */
 		rettuple = trigtuple;
 
 	SPI_finish();				/* don't forget say Bye to SPI mgr */