Merge two copies of tuple-building code in pltcl.c.

Make pltcl_trigger_handler() construct modified tuples using
pltcl_build_tuple_result(), rather than its own copy of essentially
the same logic.  This results in slightly different message wording for
the error cases, and in one case a different SQLSTATE, but it seems
unlikely that any existing applications are depending on any of those
details.

While at it, fix a typo in commit 26abb50c4: pltcl_build_tuple_result was
applying encoding conversion in the wrong direction.  That would be a
back-patchable bug fix, except the code hasn't shipped yet.

Jim Nasby, reviewed by me

Discussion: https://postgr.es/m/d2c6425a-d9e0-f034-f774-4a872c234d89@BlueTreble.com

diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 5cb4ee85e0045952be3511b838f735be2ba0300c..d813dcb3a61b590276b193c6bb3107516c3fca2c 100644 (file)
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -202,6 +202,9 @@ typedef struct pltcl_call_state
    /* Call info struct, or NULL in a trigger */
    FunctionCallInfo fcinfo;
 
+   /* Trigger data, if we're in a normal (not event) trigger; else NULL */
+   TriggerData *trigdata;
+
    /* Function we're executing (NULL if not yet identified) */
    pltcl_proc_desc *prodesc;
 
@@ -1000,8 +1003,8 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, pltcl_call_state *call_state,
    const char *result;
    int         result_Objc;
    Tcl_Obj   **result_Objv;
-   Datum      *values;
-   bool       *nulls;
+
+   call_state->trigdata = trigdata;
 
    /* Connect to SPI manager */
    if (SPI_connect() != SPI_OK_CONNECT)
@@ -1018,7 +1021,7 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, pltcl_call_state *call_state,
 
    interp = prodesc->interp_desc->interp;
 
-   tupdesc = trigdata->tg_relation->rd_att;
+   tupdesc = RelationGetDescr(trigdata->tg_relation);
 
    /************************************************************
     * Create the tcl command to call the internal
@@ -1219,69 +1222,9 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, pltcl_call_state *call_state,
                 errmsg("could not split return value from trigger: %s",
                        utf_u2e(Tcl_GetStringResult(interp)))));
 
-   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")));
-
-   values = (Datum *) palloc0(tupdesc->natts * sizeof(Datum));
-   nulls = (bool *) palloc(tupdesc->natts * sizeof(bool));
-   memset(nulls, true, tupdesc->natts * sizeof(bool));
-
-   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;
-
-       /************************************************************
-        * 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)));
-
-       /************************************************************
-        * Lookup the attribute type's input function
-        ************************************************************/
-       getTypeInputInfo(tupdesc->attrs[attnum - 1]->atttypid,
-                        &typinput, &typioparam);
-       fmgr_info(typinput, &finfo);
-
-       /************************************************************
-        * 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;
-   }
-
-   /* Build the modified tuple to return */
-   rettup = heap_form_tuple(tupdesc, values, nulls);
-
-   pfree(values);
-   pfree(nulls);
+   /* Convert function result to tuple */
+   rettup = pltcl_build_tuple_result(interp, result_Objv, result_Objc,
+                                     call_state);
 
    return rettup;
 }
@@ -3014,6 +2957,8 @@ pltcl_build_tuple_argument(HeapTuple tuple, TupleDesc tupdesc)
  * pltcl_build_tuple_result() - Build a tuple of function's result rowtype
  *               from a Tcl list of column names and values
  *
+ * In a trigger function, we build a tuple of the trigger table's rowtype.
+ *
  * Note: this function leaks memory.  Even if we made it clean up its own
  * mess, there's no way to prevent the datatype input functions it calls
  * from leaking.  Run it in a short-lived context, unless we're about to
@@ -3023,24 +2968,44 @@ static HeapTuple
 pltcl_build_tuple_result(Tcl_Interp *interp, Tcl_Obj **kvObjv, int kvObjc,
                         pltcl_call_state *call_state)
 {
+   TupleDesc   tupdesc;
+   AttInMetadata *attinmeta;
    char      **values;
    int         i;
 
+   if (call_state->ret_tupdesc)
+   {
+       tupdesc = call_state->ret_tupdesc;
+       attinmeta = call_state->attinmeta;
+   }
+   else if (call_state->trigdata)
+   {
+       tupdesc = RelationGetDescr(call_state->trigdata->tg_relation);
+       attinmeta = TupleDescGetAttInMetadata(tupdesc);
+   }
+   else
+   {
+       elog(ERROR, "PL/Tcl function does not return a tuple");
+       tupdesc = NULL;         /* keep compiler quiet */
+       attinmeta = NULL;
+   }
+
+   values = (char **) palloc0(tupdesc->natts * sizeof(char *));
+
    if (kvObjc % 2 != 0)
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
        errmsg("column name/value list must have even number of elements")));
 
-   values = (char **) palloc0(call_state->ret_tupdesc->natts * sizeof(char *));
-
    for (i = 0; i < kvObjc; i += 2)
    {
-       char       *fieldName = utf_e2u(Tcl_GetString(kvObjv[i]));
-       int         attn = SPI_fnumber(call_state->ret_tupdesc, fieldName);
+       char       *fieldName = utf_u2e(Tcl_GetString(kvObjv[i]));
+       int         attn = SPI_fnumber(tupdesc, fieldName);
 
        /*
-        * As in pltcl_trigger_handler, silently ignore ".tupno" if it's in
-        * the list but doesn't match any column name.
+        * 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().
         */
        if (attn == SPI_ERROR_NOATTRIBUTE)
        {
@@ -3058,10 +3023,10 @@ pltcl_build_tuple_result(Tcl_Interp *interp, Tcl_Obj **kvObjv, int kvObjc,
                     errmsg("cannot set system attribute \"%s\"",
                            fieldName)));
 
-       values[attn - 1] = utf_e2u(Tcl_GetString(kvObjv[i + 1]));
+       values[attn - 1] = utf_u2e(Tcl_GetString(kvObjv[i + 1]));
    }
 
-   return BuildTupleFromCStrings(call_state->attinmeta, values);
+   return BuildTupleFromCStrings(attinmeta, values);
 }
 
 /**********************************************************************