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.

diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 87113f0fb11a4e3e2b2a463597ad9967eb485f9c..4d993e7371da43bce0caa71e7c7a070b03cb246b 100644 (file)
--- 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;
 }