Modernize result-tuple construction in pltcl_trigger_handler().

Use Tcl_ListObjGetElements instead of Tcl_SplitList.  Aside from being
possibly more efficient in its own right, this means we are no longer
responsible for freeing a malloc'd result array, so we can get rid of
a PG_TRY/PG_CATCH block.

Use heap_form_tuple instead of SPI_modifytuple.  We don't need the
extra generality of the latter, since we're always replacing all
columns.  Nor do we need its memory-context-munging, since at this
point we're already out of the SPI environment.

Per comparison of this code to tuple-building code submitted by Jim Nasby.
I've abandoned the thought of merging the two cases into a single routine,
but we may as well make the older code simpler and faster where we can.

diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 44fcf68054f718a37010284c032a8bda62e5dbab..97d1f7ef7d3746936c8e68f91ebc5b2f3f5aa60c 100644 (file)
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -870,12 +870,11 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, bool pltrusted)
        Tcl_Obj    *tcl_newtup;
        int                     tcl_rc;
        int                     i;
-       int                *modattrs;
-       Datum      *modvalues;
-       char       *modnulls;
-       int                     ret_numvals;
        const char *result;
-       const char **ret_values;
+       int                     result_Objc;
+       Tcl_Obj   **result_Objv;
+       Datum      *values;
+       bool       *nulls;
 
        /* Connect to SPI manager */
        if (SPI_connect() != SPI_OK_CONNECT)
@@ -1065,13 +1064,16 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, bool pltrusted)
                throw_tcl_error(interp, prodesc->user_proname);
 
        /************************************************************
-        * The return value from the procedure might be one of
-        * the magic strings OK or SKIP or a list from array get.
-        * We can check for OK or SKIP without worrying about encoding.
+        * Exit SPI environment.
         ************************************************************/
        if (SPI_finish() != SPI_OK_FINISH)
                elog(ERROR, "SPI_finish() failed");
 
+       /************************************************************
+        * The return value from the procedure might be one of
+        * the magic strings OK or SKIP, or a list from array get.
+        * We can check for OK or SKIP without worrying about encoding.
+        ************************************************************/
        result = Tcl_GetStringResult(interp);
 
        if (strcmp(result, "OK") == 0)
@@ -1080,108 +1082,85 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, bool pltrusted)
                return (HeapTuple) NULL;
 
        /************************************************************
-        * Convert the result value from the Tcl interpreter
-        * and setup structures for SPI_modifytuple();
+        * Otherwise, the return value should be a column name/value list
+        * specifying the modified tuple to return.
         ************************************************************/
-       if (Tcl_SplitList(interp, result,
-                                         &ret_numvals, &ret_values) != TCL_OK)
+       if (Tcl_ListObjGetElements(interp, Tcl_GetObjResult(interp),
+                                                          &result_Objc, &result_Objv) != TCL_OK)
                ereport(ERROR,
                                (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
                                 errmsg("could not split return value from trigger: %s",
                                                utf_u2e(Tcl_GetStringResult(interp)))));
 
-       /* Use a TRY to ensure ret_values will get freed */
-       PG_TRY();
-       {
-               if (ret_numvals % 2 != 0)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
-                                        errmsg("trigger's return list must have even number of elements")));
+       if (result_Objc % 2 != 0)
+               ereport(ERROR,
+                               (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+                errmsg("trigger's return list must have even number of elements")));
 
-               modattrs = (int *) palloc(tupdesc->natts * sizeof(int));
-               modvalues = (Datum *) palloc(tupdesc->natts * sizeof(Datum));
-               for (i = 0; i < tupdesc->natts; i++)
-               {
-                       modattrs[i] = i + 1;
-                       modvalues[i] = (Datum) NULL;
-               }
+       values = (Datum *) palloc0(tupdesc->natts * sizeof(Datum));
+       nulls = (bool *) palloc(tupdesc->natts * sizeof(bool));
+       memset(nulls, true, tupdesc->natts * sizeof(bool));
 
-               modnulls = palloc(tupdesc->natts);
-               memset(modnulls, 'n', tupdesc->natts);
+       for (i = 0; i < result_Objc; i += 2)
+       {
+               char       *ret_name = utf_u2e(Tcl_GetString(result_Objv[i]));
+               char       *ret_value = utf_u2e(Tcl_GetString(result_Objv[i + 1]));
+               int                     attnum;
+               Oid                     typinput;
+               Oid                     typioparam;
+               FmgrInfo        finfo;
 
-               for (i = 0; i < ret_numvals; i += 2)
+               /************************************************************
+                * Get the attribute number
+                *
+                * We silently ignore ".tupno", if it's present but doesn't match
+                * any actual output column.  This allows direct use of a row
+                * returned by pltcl_set_tuple_values().
+                ************************************************************/
+               attnum = SPI_fnumber(tupdesc, ret_name);
+               if (attnum == SPI_ERROR_NOATTRIBUTE)
                {
-                       char       *ret_name = utf_u2e(ret_values[i]);
-                       char       *ret_value = utf_u2e(ret_values[i + 1]);
-                       int                     attnum;
-                       Oid                     typinput;
-                       Oid                     typioparam;
-                       FmgrInfo        finfo;
-
-                       /************************************************************
-                        * Get the attribute number
-                        *
-                        * We silently ignore ".tupno", if it's present but doesn't match
-                        * any actual output column.  This allows direct use of a row
-                        * returned by pltcl_set_tuple_values().
-                        ************************************************************/
-                       attnum = SPI_fnumber(tupdesc, ret_name);
-                       if (attnum == SPI_ERROR_NOATTRIBUTE)
-                       {
-                               if (strcmp(ret_name, ".tupno") == 0)
-                                       continue;
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_UNDEFINED_COLUMN),
-                                                errmsg("unrecognized attribute \"%s\"",
-                                                               ret_name)));
-                       }
-                       if (attnum <= 0)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                                errmsg("cannot set system attribute \"%s\"",
-                                                               ret_name)));
-
-                       /************************************************************
-                        * Ignore dropped columns
-                        ************************************************************/
-                       if (tupdesc->attrs[attnum - 1]->attisdropped)
+                       if (strcmp(ret_name, ".tupno") == 0)
                                continue;
-
-                       /************************************************************
-                        * Lookup the attribute type in the syscache
-                        * for the input function
-                        ************************************************************/
-                       getTypeInputInfo(tupdesc->attrs[attnum - 1]->atttypid,
-                                                        &typinput, &typioparam);
-                       fmgr_info(typinput, &finfo);
-
-                       /************************************************************
-                        * Set the attribute to NOT NULL and convert the contents
-                        ************************************************************/
-                       modvalues[attnum - 1] = InputFunctionCall(&finfo,
-                                                                                                         ret_value,
-                                                                                                         typioparam,
-                                                                         tupdesc->attrs[attnum - 1]->atttypmod);
-                       modnulls[attnum - 1] = ' ';
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_UNDEFINED_COLUMN),
+                                        errmsg("unrecognized attribute \"%s\"",
+                                                       ret_name)));
                }
+               if (attnum <= 0)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                        errmsg("cannot set system attribute \"%s\"",
+                                                       ret_name)));
 
-               rettup = SPI_modifytuple(trigdata->tg_relation, rettup, tupdesc->natts,
-                                                                modattrs, modvalues, modnulls);
+               /************************************************************
+                * Ignore dropped columns
+                ************************************************************/
+               if (tupdesc->attrs[attnum - 1]->attisdropped)
+                       continue;
 
-               pfree(modattrs);
-               pfree(modvalues);
-               pfree(modnulls);
+               /************************************************************
+                * Lookup the attribute type's input function
+                ************************************************************/
+               getTypeInputInfo(tupdesc->attrs[attnum - 1]->atttypid,
+                                                &typinput, &typioparam);
+               fmgr_info(typinput, &finfo);
 
-               if (rettup == NULL)
-                       elog(ERROR, "SPI_modifytuple() failed - RC = %d", SPI_result);
-       }
-       PG_CATCH();
-       {
-               ckfree((char *) ret_values);
-               PG_RE_THROW();
+               /************************************************************
+                * Set the attribute to NOT NULL and convert the contents
+                ************************************************************/
+               values[attnum - 1] = InputFunctionCall(&finfo,
+                                                                                          ret_value,
+                                                                                          typioparam,
+                                                                         tupdesc->attrs[attnum - 1]->atttypmod);
+               nulls[attnum - 1] = false;
        }
-       PG_END_TRY();
-       ckfree((char *) ret_values);
+
+       /* Build the modified tuple to return */
+       rettup = heap_form_tuple(tupdesc, values, nulls);
+
+       pfree(values);
+       pfree(nulls);
 
        return rettup;
 }