Use heap_modify_tuple not SPI_modifytuple in pl/perl triggers.

The code here would need some change anyway given planned change in
SPI_modifytuple semantics, since this executes after we've exited the
SPI environment.  But really it's better to just use heap_modify_tuple.
The code's actually shorter this way, and this avoids depending on some
rather indirect reasoning about why the temporary arrays can't be overrun.
(I think the old code is safe, as long as Perl hashes can't contain
duplicate keys; but with this way we don't need that assumption, only
the assumption that SPI_fnumber doesn't return an out-of-range attnum.)

While at it, normalize use of SPI_fnumber: make error messages distinguish
no-such-column from can't-set-system-column, and remove test for deleted
column which is going to migrate into SPI_fnumber.

1 files changed, 25 insertions, 31 deletions

diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 87113f0fb11..4d993e7371d 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -1670,8 +1670,7 @@ plperl_event_trigger_build_args(FunctionCallInfo fcinfo)
 	return newRV_noinc((SV *) hv);
 }
 
-/* Set up the new tuple returned from a trigger. */
-
+/* Construct the modified new tuple to be returned from a trigger. */
 static HeapTuple
 plperl_modify_tuple(HV *hvTD, TriggerData *tdata, HeapTuple otup)
 {
@@ -1679,14 +1678,11 @@ plperl_modify_tuple(HV *hvTD, TriggerData *tdata, HeapTuple otup)
 	HV		   *hvNew;
 	HE		   *he;
 	HeapTuple	rtup;
-	int			slotsused;
-	int		   *modattrs;
-	Datum	   *modvalues;
-	char	   *modnulls;
-
 	TupleDesc	tupdesc;
-
-	tupdesc = tdata->tg_relation->rd_att;
+	int			natts;
+	Datum	   *modvalues;
+	bool	   *modnulls;
+	bool	   *modrepls;
 
 	svp = hv_fetch_string(hvTD, "new");
 	if (!svp)
@@ -1699,51 +1695,49 @@ plperl_modify_tuple(HV *hvTD, TriggerData *tdata, HeapTuple otup)
 				 errmsg("$_TD->{new} is not a hash reference")));
 	hvNew = (HV *) SvRV(*svp);
 
-	modattrs = palloc(tupdesc->natts * sizeof(int));
-	modvalues = palloc(tupdesc->natts * sizeof(Datum));
-	modnulls = palloc(tupdesc->natts * sizeof(char));
-	slotsused = 0;
+	tupdesc = tdata->tg_relation->rd_att;
+	natts = tupdesc->natts;
+
+	modvalues = (Datum *) palloc0(natts * sizeof(Datum));
+	modnulls = (bool *) palloc0(natts * sizeof(bool));
+	modrepls = (bool *) palloc0(natts * sizeof(bool));
 
 	hv_iterinit(hvNew);
 	while ((he = hv_iternext(hvNew)))
 	{
-		bool		isnull;
 		char	   *key = hek2cstr(he);
 		SV		   *val = HeVAL(he);
 		int			attn = SPI_fnumber(tupdesc, key);
 
-		if (attn <= 0 || tupdesc->attrs[attn - 1]->attisdropped)
+		if (attn == SPI_ERROR_NOATTRIBUTE)
 			ereport(ERROR,
 					(errcode(ERRCODE_UNDEFINED_COLUMN),
 					 errmsg("Perl hash contains nonexistent column \"%s\"",
 							key)));
+		if (attn <= 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot set system attribute \"%s\"",
+							key)));
 
-		modvalues[slotsused] = plperl_sv_to_datum(val,
+		modvalues[attn - 1] = plperl_sv_to_datum(val,
 										  tupdesc->attrs[attn - 1]->atttypid,
 										 tupdesc->attrs[attn - 1]->atttypmod,
-												  NULL,
-												  NULL,
-												  InvalidOid,
-												  &isnull);
-
-		modnulls[slotsused] = isnull ? 'n' : ' ';
-		modattrs[slotsused] = attn;
-		slotsused++;
+												 NULL,
+												 NULL,
+												 InvalidOid,
+												 &modnulls[attn - 1]);
+		modrepls[attn - 1] = true;
 
 		pfree(key);
 	}
 	hv_iterinit(hvNew);
 
-	rtup = SPI_modifytuple(tdata->tg_relation, otup, slotsused,
-						   modattrs, modvalues, modnulls);
+	rtup = heap_modify_tuple(otup, tupdesc, modvalues, modnulls, modrepls);
 
-	pfree(modattrs);
 	pfree(modvalues);
 	pfree(modnulls);
-
-	if (rtup == NULL)
-		elog(ERROR, "SPI_modifytuple failed: %s",
-			 SPI_result_code_string(SPI_result));
+	pfree(modrepls);
 
 	return rtup;
 }