Re: [HACKERS] statement level trigger causes pltcl, plpython - Mailing list pgsql-patches

From Joe Conway
Subject Re: [HACKERS] statement level trigger causes pltcl, plpython
Date
Msg-id 3F2E93AA.4090208@joeconway.com
Whole thread Raw
In response to Re: [HACKERS] statement level trigger causes pltcl, plpython SIGSEGV  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] statement level trigger causes pltcl, plpython SIGSEGV
List pgsql-patches
Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>
>>Note that I also changed behavior in that when trigdata->tg_event
>>doesn't match anything known -- instead of pressing on with a value of
>>"UNKNOWN" it now throws an "elog(ERROR...". The previous behavior made
>>no sense to me, but you may not want to change existing behavior in this
>>way (even though it seems to me that it is a "should not happen" kind of
>>error).
>
> Seems reasonable to me.  As a matter of style, the "unrecognized" elogs
> should print the offending tg_event value; otherwise the patch looks fine.
>
>>If this patch is acceptable, I'll make similar changes to plpython.
>
> Please do.
>

This patch includes pltcl and plpython, with the mentioned style issue
fixed. Both PLs pass their scripted tests and my simple statement level
trigger test.

Joe
Index: src/pl/plpython/plpython.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/pl/plpython/plpython.c,v
retrieving revision 1.38
diff -c -r1.38 plpython.c
*** src/pl/plpython/plpython.c    4 Aug 2003 01:57:58 -0000    1.38
--- src/pl/plpython/plpython.c    4 Aug 2003 17:32:55 -0000
***************
*** 612,618 ****
      DECLARE_EXC();
      TriggerData *tdata;
      PyObject   *pltname,
!                *pltevent,
                 *pltwhen,
                 *pltlevel,
                 *pltrelid;
--- 612,618 ----
      DECLARE_EXC();
      TriggerData *tdata;
      PyObject   *pltname,
!                *pltevent = NULL,
                 *pltwhen,
                 *pltlevel,
                 *pltrelid;
***************
*** 651,718 ****
      Py_DECREF(pltrelid);
      pfree(stroid);

-
-
      if (TRIGGER_FIRED_BEFORE(tdata->tg_event))
          pltwhen = PyString_FromString("BEFORE");
      else if (TRIGGER_FIRED_AFTER(tdata->tg_event))
          pltwhen = PyString_FromString("AFTER");
      else
!         pltwhen = PyString_FromString("UNKNOWN");
      PyDict_SetItemString(pltdata, "when", pltwhen);
      Py_DECREF(pltwhen);

      if (TRIGGER_FIRED_FOR_ROW(tdata->tg_event))
          pltlevel = PyString_FromString("ROW");
      else if (TRIGGER_FIRED_FOR_STATEMENT(tdata->tg_event))
          pltlevel = PyString_FromString("STATEMENT");
!     else
!         pltlevel = PyString_FromString("UNKNOWN");
!     PyDict_SetItemString(pltdata, "level", pltlevel);
!     Py_DECREF(pltlevel);

-     if (TRIGGER_FIRED_BY_INSERT(tdata->tg_event))
-     {
-         pltevent = PyString_FromString("INSERT");
          PyDict_SetItemString(pltdata, "old", Py_None);
-         pytnew = PLyDict_FromTuple(&(proc->result), tdata->tg_trigtuple,
-                                    tdata->tg_relation->rd_att);
-         PyDict_SetItemString(pltdata, "new", pytnew);
-         Py_DECREF(pytnew);
-         *rv = tdata->tg_trigtuple;
-     }
-     else if (TRIGGER_FIRED_BY_DELETE(tdata->tg_event))
-     {
-         pltevent = PyString_FromString("DELETE");
          PyDict_SetItemString(pltdata, "new", Py_None);
!         pytold = PLyDict_FromTuple(&(proc->result), tdata->tg_trigtuple,
!                                    tdata->tg_relation->rd_att);
!         PyDict_SetItemString(pltdata, "old", pytold);
!         Py_DECREF(pytold);
!         *rv = tdata->tg_trigtuple;
!     }
!     else if (TRIGGER_FIRED_BY_UPDATE(tdata->tg_event))
!     {
!         pltevent = PyString_FromString("UPDATE");
!         pytnew = PLyDict_FromTuple(&(proc->result), tdata->tg_newtuple,
!                                    tdata->tg_relation->rd_att);
!         PyDict_SetItemString(pltdata, "new", pytnew);
!         Py_DECREF(pytnew);
!         pytold = PLyDict_FromTuple(&(proc->result), tdata->tg_trigtuple,
!                                    tdata->tg_relation->rd_att);
!         PyDict_SetItemString(pltdata, "old", pytold);
!         Py_DECREF(pytold);
!         *rv = tdata->tg_newtuple;
      }
      else
!     {
!         pltevent = PyString_FromString("UNKNOWN");
!         PyDict_SetItemString(pltdata, "old", Py_None);
!         PyDict_SetItemString(pltdata, "new", Py_None);
!         *rv = tdata->tg_trigtuple;
!     }
!     PyDict_SetItemString(pltdata, "event", pltevent);
!     Py_DECREF(pltevent);

      if (tdata->tg_trigger->tgnargs)
      {
--- 651,741 ----
      Py_DECREF(pltrelid);
      pfree(stroid);

      if (TRIGGER_FIRED_BEFORE(tdata->tg_event))
          pltwhen = PyString_FromString("BEFORE");
      else if (TRIGGER_FIRED_AFTER(tdata->tg_event))
          pltwhen = PyString_FromString("AFTER");
      else
!         /* internal error */
!         elog(ERROR, "unrecognized WHEN tg_event: %u", tdata->tg_event);
      PyDict_SetItemString(pltdata, "when", pltwhen);
      Py_DECREF(pltwhen);

      if (TRIGGER_FIRED_FOR_ROW(tdata->tg_event))
+     {
          pltlevel = PyString_FromString("ROW");
+         PyDict_SetItemString(pltdata, "level", pltlevel);
+         Py_DECREF(pltlevel);
+
+         if (TRIGGER_FIRED_BY_INSERT(tdata->tg_event))
+         {
+             pltevent = PyString_FromString("INSERT");
+
+             PyDict_SetItemString(pltdata, "old", Py_None);
+             pytnew = PLyDict_FromTuple(&(proc->result), tdata->tg_trigtuple,
+                                        tdata->tg_relation->rd_att);
+             PyDict_SetItemString(pltdata, "new", pytnew);
+             Py_DECREF(pytnew);
+             *rv = tdata->tg_trigtuple;
+         }
+         else if (TRIGGER_FIRED_BY_DELETE(tdata->tg_event))
+         {
+             pltevent = PyString_FromString("DELETE");
+
+             PyDict_SetItemString(pltdata, "new", Py_None);
+             pytold = PLyDict_FromTuple(&(proc->result), tdata->tg_trigtuple,
+                                        tdata->tg_relation->rd_att);
+             PyDict_SetItemString(pltdata, "old", pytold);
+             Py_DECREF(pytold);
+             *rv = tdata->tg_trigtuple;
+         }
+         else if (TRIGGER_FIRED_BY_UPDATE(tdata->tg_event))
+         {
+             pltevent = PyString_FromString("UPDATE");
+
+             pytnew = PLyDict_FromTuple(&(proc->result), tdata->tg_newtuple,
+                                        tdata->tg_relation->rd_att);
+             PyDict_SetItemString(pltdata, "new", pytnew);
+             Py_DECREF(pytnew);
+             pytold = PLyDict_FromTuple(&(proc->result), tdata->tg_trigtuple,
+                                        tdata->tg_relation->rd_att);
+             PyDict_SetItemString(pltdata, "old", pytold);
+             Py_DECREF(pytold);
+             *rv = tdata->tg_newtuple;
+         }
+         else
+             /* internal error */
+             elog(ERROR, "unrecognized OP tg_event: %u", tdata->tg_event);
+
+         PyDict_SetItemString(pltdata, "event", pltevent);
+         Py_DECREF(pltevent);
+     }
      else if (TRIGGER_FIRED_FOR_STATEMENT(tdata->tg_event))
+     {
          pltlevel = PyString_FromString("STATEMENT");
!         PyDict_SetItemString(pltdata, "level", pltlevel);
!         Py_DECREF(pltlevel);

          PyDict_SetItemString(pltdata, "old", Py_None);
          PyDict_SetItemString(pltdata, "new", Py_None);
!         *rv = (HeapTuple) NULL;
!
!         if (TRIGGER_FIRED_BY_INSERT(tdata->tg_event))
!             pltevent = PyString_FromString("INSERT");
!         else if (TRIGGER_FIRED_BY_DELETE(tdata->tg_event))
!             pltevent = PyString_FromString("DELETE");
!         else if (TRIGGER_FIRED_BY_UPDATE(tdata->tg_event))
!             pltevent = PyString_FromString("UPDATE");
!         else
!             /* internal error */
!             elog(ERROR, "unrecognized OP tg_event: %u", tdata->tg_event);
!
!         PyDict_SetItemString(pltdata, "event", pltevent);
!         Py_DECREF(pltevent);
      }
      else
!         /* internal error */
!         elog(ERROR, "unrecognized LEVEL tg_event: %u", tdata->tg_event);

      if (tdata->tg_trigger->tgnargs)
      {
Index: src/pl/tcl/pltcl.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/pl/tcl/pltcl.c,v
retrieving revision 1.74
diff -c -r1.74 pltcl.c
*** src/pl/tcl/pltcl.c    4 Aug 2003 00:43:33 -0000    1.74
--- src/pl/tcl/pltcl.c    4 Aug 2003 17:07:24 -0000
***************
*** 708,770 ****
      else if (TRIGGER_FIRED_AFTER(trigdata->tg_event))
          Tcl_DStringAppendElement(&tcl_cmd, "AFTER");
      else
!         Tcl_DStringAppendElement(&tcl_cmd, "UNKNOWN");

      /* The level part of the event for TG_level */
      if (TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
          Tcl_DStringAppendElement(&tcl_cmd, "ROW");
      else if (TRIGGER_FIRED_FOR_STATEMENT(trigdata->tg_event))
          Tcl_DStringAppendElement(&tcl_cmd, "STATEMENT");
-     else
-         Tcl_DStringAppendElement(&tcl_cmd, "UNKNOWN");

!     /* Build the data list for the trigtuple */
!     pltcl_build_tuple_argument(trigdata->tg_trigtuple,
!                                tupdesc, &tcl_trigtup);
!
!     /*
!      * Now the command part of the event for TG_op and data for NEW and
!      * OLD
!      */
!     if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event))
!     {
!         Tcl_DStringAppendElement(&tcl_cmd, "INSERT");

-         Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
          Tcl_DStringAppendElement(&tcl_cmd, "");
-
-         rettup = trigdata->tg_trigtuple;
-     }
-     else if (TRIGGER_FIRED_BY_DELETE(trigdata->tg_event))
-     {
-         Tcl_DStringAppendElement(&tcl_cmd, "DELETE");
-
          Tcl_DStringAppendElement(&tcl_cmd, "");
-         Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));

!         rettup = trigdata->tg_trigtuple;
!     }
!     else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
!     {
!         Tcl_DStringAppendElement(&tcl_cmd, "UPDATE");
!
!         pltcl_build_tuple_argument(trigdata->tg_newtuple,
!                                    tupdesc, &tcl_newtup);
!
!         Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_newtup));
!         Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
!
!         rettup = trigdata->tg_newtuple;
      }
      else
!     {
!         Tcl_DStringAppendElement(&tcl_cmd, "UNKNOWN");
!
!         Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
!         Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
!
!         rettup = trigdata->tg_trigtuple;
!     }

      memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
      Tcl_DStringFree(&tcl_trigtup);
--- 708,785 ----
      else if (TRIGGER_FIRED_AFTER(trigdata->tg_event))
          Tcl_DStringAppendElement(&tcl_cmd, "AFTER");
      else
!         /* internal error */
!         elog(ERROR, "unrecognized WHEN tg_event: %u", trigdata->tg_event);

      /* The level part of the event for TG_level */
      if (TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
+     {
          Tcl_DStringAppendElement(&tcl_cmd, "ROW");
+
+         /* Build the data list for the trigtuple */
+         pltcl_build_tuple_argument(trigdata->tg_trigtuple,
+                                    tupdesc, &tcl_trigtup);
+
+         /*
+          * Now the command part of the event for TG_op and data for NEW and
+          * OLD
+          */
+         if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event))
+         {
+             Tcl_DStringAppendElement(&tcl_cmd, "INSERT");
+
+             Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
+             Tcl_DStringAppendElement(&tcl_cmd, "");
+
+             rettup = trigdata->tg_trigtuple;
+         }
+         else if (TRIGGER_FIRED_BY_DELETE(trigdata->tg_event))
+         {
+             Tcl_DStringAppendElement(&tcl_cmd, "DELETE");
+
+             Tcl_DStringAppendElement(&tcl_cmd, "");
+             Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
+
+             rettup = trigdata->tg_trigtuple;
+         }
+         else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
+         {
+             Tcl_DStringAppendElement(&tcl_cmd, "UPDATE");
+
+             pltcl_build_tuple_argument(trigdata->tg_newtuple,
+                                        tupdesc, &tcl_newtup);
+
+             Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_newtup));
+             Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
+
+             rettup = trigdata->tg_newtuple;
+         }
+         else
+             /* internal error */
+             elog(ERROR, "unrecognized OP tg_event: %u", trigdata->tg_event);
+     }
      else if (TRIGGER_FIRED_FOR_STATEMENT(trigdata->tg_event))
+     {
          Tcl_DStringAppendElement(&tcl_cmd, "STATEMENT");

!         if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event))
!             Tcl_DStringAppendElement(&tcl_cmd, "INSERT");
!         else if (TRIGGER_FIRED_BY_DELETE(trigdata->tg_event))
!             Tcl_DStringAppendElement(&tcl_cmd, "DELETE");
!         else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
!             Tcl_DStringAppendElement(&tcl_cmd, "UPDATE");
!         else
!             /* internal error */
!             elog(ERROR, "unrecognized OP tg_event: %u", trigdata->tg_event);

          Tcl_DStringAppendElement(&tcl_cmd, "");
          Tcl_DStringAppendElement(&tcl_cmd, "");

!         rettup = (HeapTuple) NULL;
      }
      else
!         /* internal error */
!         elog(ERROR, "unrecognized LEVEL tg_event: %u", trigdata->tg_event);

      memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
      Tcl_DStringFree(&tcl_trigtup);

pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: [HACKERS] statement level trigger causes pltcl, plpython SIGSEGV
Next
From: Tom Lane
Date:
Subject: Re: [HACKERS] statement level trigger causes pltcl, plpython SIGSEGV