From 0d4446083df56a36ecec8e8bd321a45ecac7e7c6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 8 Nov 2016 11:35:01 -0500 Subject: [PATCH] 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. --- src/pl/plperl/plperl.c | 56 +++++++++++++++++++----------------------- 1 file 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; }