Re: plpython tracebacks - Mailing list pgsql-patches

P. Scott DeVos wrote:
> I'm on it.

Actually, don't worry about it -- I've made the corrections I had in
mind myself. Attached is a revised patch. On looking closer, I didn't
really like the way the patch accumulated the lines of the traceback:
AFAICS _PyString_Join() is not an "official" Python C API function (it's
not documented, at any rate), and besides it is cleaner and more
efficient to build up the traceback string in a StringInfo rather than
using Python lists and strings.

The attached patch isn't quite finished: "No Traceback" when there is no
traceback information doesn't seem like the best message, I need to
update the regression tests and some comments, etc. But I plan to apply
something similar in substance to the attached patch to HEAD in the next
day or two, barring objections.

-Neil
Index: src/pl/plpython/plpython.c
===================================================================
RCS file: /Users/neilc/postgres/cvs_root/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.71
diff -c -r1.71 plpython.c
*** src/pl/plpython/plpython.c    20 Feb 2006 20:10:37 -0000    1.71
--- src/pl/plpython/plpython.c    22 Feb 2006 22:36:29 -0000
***************
*** 48,53 ****
--- 48,54 ----
  #include "commands/trigger.h"
  #include "executor/spi.h"
  #include "fmgr.h"
+ #include "lib/stringinfo.h"
  #include "nodes/makefuncs.h"
  #include "parser/parse_type.h"
  #include "tcop/tcopprot.h"
***************
*** 2489,2494 ****
--- 2490,2534 ----
  }

  static char *
+ build_python_traceback(PyObject *tb)
+ {
+     PyObject *cur_tb;
+     StringInfoData buf;
+
+     initStringInfo(&buf);
+     appendStringInfoString(&buf, "Traceback (most recent call last):\n");
+
+     /* skip the first element in the traceback list */
+     cur_tb = PyObject_GetAttrString(tb, "tb_next");
+     while (cur_tb != Py_None)
+     {
+         PyObject *lno = PyObject_GetAttrString(cur_tb, "tb_lineno");
+         PyObject *frame = PyObject_GetAttrString(cur_tb, "tb_frame");
+         PyObject *code = PyObject_GetAttrString(frame, "f_code");
+         PyObject *file = PyObject_GetAttrString(code, "co_filename");
+         PyObject *name = PyObject_GetAttrString(code, "co_name");
+         PyObject *prev_tb;
+
+         appendStringInfo(&buf, "  File \"%s\", line %s, in %s\n",
+                          PyString_AsString(file),
+                          PyString_AsString(lno),
+                          PyString_AsString(name));
+
+         Py_DECREF(lno);
+         Py_DECREF(frame);
+         Py_DECREF(code);
+         Py_DECREF(file);
+         Py_DECREF(name);
+
+         prev_tb = cur_tb;
+         cur_tb = PyObject_GetAttrString(cur_tb, "tb_next");
+         Py_DECREF(prev_tb);
+     }
+
+     return buf.data;
+ }
+
+ static char *
  PLy_traceback(int *xlevel)
  {
      PyObject   *e,
***************
*** 2498,2503 ****
--- 2538,2544 ----
                 *vob = NULL;
      char       *vstr,
                 *estr,
+                *tbstr,
                 *xstr = NULL;

      /*
***************
*** 2515,2521 ****
      }

      PyErr_NormalizeException(&e, &v, &tb);
-     Py_XDECREF(tb);

      eob = PyObject_Str(e);
      if (v && ((vob = PyObject_Str(v)) != NULL))
--- 2556,2561 ----
***************
*** 2524,2540 ****
          vstr = "Unknown";

      /*
       * I'm not sure what to do if eob is NULL here -- we can't call PLy_elog
       * because that function calls us, so we could end up with infinite
       * recursion.  I'm not even sure if eob could be NULL here -- would an
       * Assert() be more appropriate?
       */
      estr = eob ? PyString_AsString(eob) : "Unknown Exception";
!     xstr = PLy_printf("%s: %s", estr, vstr);

      Py_DECREF(eob);
      Py_XDECREF(vob);
      Py_XDECREF(v);

      /*
       * intuit an appropriate error level based on the exception type
--- 2564,2594 ----
          vstr = "Unknown";

      /*
+      * If there is a traceback object, build a string containing a
+      * textual representation of the traceback.
+      */
+     if (tb)
+     {
+         tbstr = build_python_traceback(tb);
+         /* we're done with the traceback object itself now */
+         Py_DECREF(tb);
+     }
+     else
+         tbstr = pstrdup("No Traceback\n");
+
+     /*
       * I'm not sure what to do if eob is NULL here -- we can't call PLy_elog
       * because that function calls us, so we could end up with infinite
       * recursion.  I'm not even sure if eob could be NULL here -- would an
       * Assert() be more appropriate?
       */
      estr = eob ? PyString_AsString(eob) : "Unknown Exception";
!     xstr = PLy_printf("%s%s: %s", tbstr, estr, vstr);

      Py_DECREF(eob);
      Py_XDECREF(vob);
      Py_XDECREF(v);
+     pfree(tbstr);

      /*
       * intuit an appropriate error level based on the exception type