Re: [PATCHES] plpython tracebacks
URL added to TODO list. --- Neil Conway wrote: 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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] plpython tracebacks
Bruce Momjian said: Neil, where are we on this patch? My apologies for forgetting about this -- there are a few details I need to finish off in the patch before I'd like to apply it. I'm on vacation at the moment and not really in a position to write any code, but I'll definitely be able to finish the patch when I return at the end of May. If you'd like to apply the patch sooner, anyone is welcome to finish the patch -- my recollection is that the only outstanding issues are updating the regression tests, improving some comments, and considering how this ought to generalize to other PLs. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] plpython tracebacks
Neil, where are we on this patch? --- Neil Conway wrote: 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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] plpython tracebacks
Neil Conway wrote: Context diffs are preferred. Also, diffs should be against the root of the source tree, and attached as MIME attachements if possible (it seems the mailing list software munges the patch somewhat otherwise). Minor nit: lowercase Append. Similarly, consistent capitalization for all the preceding comments in the block (adjacent to the variable declarations) would be nice. tbstr points into storage owned by tb_log, but the reference to tb_log has already been released. I'm on it. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plpython tracebacks
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 - 1.71
--- src/pl/plpython/plpython.c 22 Feb 2006 22:36:29 -
***
*** 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
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] plpython tracebacks
Neil Conway wrote: 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. I like it. You can tell I'm a python programmer, not a C programmer. Question: Are you sure this works: PyString_AsString(lno)? lno is a python integer object. Maybe we want, PyString_AsString(PyObject_Str(lno)) The attached patch isn't quite finished: No Traceback when there is no traceback information doesn't seem like the best message OK, how about There is no traceback information , 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. Thanks for your attention to this! ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plpython tracebacks
P. Scott DeVos wrote: Are you sure this works: PyString_AsString(lno)? lno is a python integer object. Maybe we want, PyString_AsString(PyObject_Str(lno)) Wait, this is not the way to do it because PyObject_Str returns a new reference. I think you have to assign PyObject_Str(lno) to another variable so that you can decrement the reference on it. Unless your code works as it, in which case, never mind... Scott ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] plpython tracebacks
On Mon, 2006-02-06 at 16:05 -0600, P. Scott DeVos wrote: I have been working with plpython for several months and have been hampered by the lack of a traceback being logged when a plpython function raises an error. I have written a patch causes the PLy_traceback function to fully log the traceback. The output looks just like the traceback output provided by the python interpreter. Can any PL/Python folks comment on whether they want this patch? Feedback appreciated. Context diffs are preferred. Also, diffs should be against the root of the source tree, and attached as MIME attachements if possible (it seems the mailing list software munges the patch somewhat otherwise). + hdr = PyString_FromString(Traceback (most recent call last):); + tmpl = PyString_FromString( File \%s\, line %s, in %s); + ftr = PyString_FromString(); + + tb_list = PyList_New(0); /* create the list of strings */ + PyList_Append(tb_list, hdr); /* Append the header to the list */ Minor nit: lowercase Append. Similarly, consistent capitalization for all the preceding comments in the block (adjacent to the variable declarations) would be nice. estr = eob ? PyString_AsString(eob) : Unknown Exception; - xstr = PLy_printf(%s: %s, estr, vstr); + xstr = PLy_printf(%s%s: %s, tbstr, estr, vstr); tbstr points into storage owned by tb_log, but the reference to tb_log has already been released. -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] plpython tracebacks
Neil,I have written a patch causes the PLy_traceback function to fully log the traceback.The output looks just like the traceback output provided by the python interpreter.Can any PL/Python folks comment on whether they want this patch?I am just a humble user of PL/Py, but tracebacks are very wellcome. Not qualified to judge it's code quality I am. Harald-- GHUM Harald Massapersuadere et programmareHarald Armin MassaReinsburgstraße 202b70197 Stuttgart0173/9409607
Re: [PATCHES] plpython tracebacks
On Sun, Feb 19, 2006 at 08:09:09PM -0500, Neil Conway wrote: On Mon, 2006-02-06 at 16:05 -0600, P. Scott DeVos wrote: I have been working with plpython for several months and have been hampered by the lack of a traceback being logged when a plpython function raises an error. I have written a patch causes the PLy_traceback function to fully log the traceback. The output looks just like the traceback output provided by the python interpreter. Can any PL/Python folks comment on whether they want this patch? Yes. Absolutely. This is a required feature for any serious Python user. One of the great things about python is that almost all debugging can be done with just the excellent standard python tracebacks. Not having them is a a not only a inconvience, it signals that the implementation is incomplete in major ways and, unready for real use. -dg -- David Gould [EMAIL PROTECTED] If simplicity worked, the world would be overrun with insects. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] plpython tracebacks
I have been working with plpython for several months and have
been hampered by the lack of a traceback being logged when a
plpython function raises an error. I have written a patch causes
the PLy_traceback function to fully log the traceback. The
output looks just like the traceback output provided by the
python interpreter.
Feedback appreciated.
Scott
--- plpython-1.70.c.orig2006-02-06 14:24:42.0 -0600
+++ plpython-1.70.c.patched 2006-02-06 15:34:05.0 -0600
@@ -2499,7 +2499,8 @@
*vob = NULL;
char *vstr,
*estr,
- *xstr = NULL;
+ *xstr = NULL,
+ *tbstr;
/*
* get the current exception
@@ -2523,6 +2524,82 @@
else
vstr = Unknown;
+ /* If there is a traceback object, we build a string containing
+ the traceback information. */
+ if (tb != NULL)
+ {
+ PyObject
+ *cur_tb, /* traceback (tb) item being handled */
+ *old_tb, /* holds tb so we can decrement reference to
it */
+ *hdr, /* First line of logged output */
+ *tmpl,/* PyString template for the logged tb item */
+ *ftr, /* Last line of logged output */
+ *tb_list, /* Each tb item create a PyString in this
list */
+ *ln, /* The line number of the item in the
traceback */
+ *frame, /* the tb_frame */
+ *code,/* the f_code this guy has filename and
method name*/
+ *fn, /* the filename of the item in the tb */
+ *nm, /* the function/method name of the item in
the tb */
+ *args,/* A tuple of the form (fn, ln, nm) */
+ *logline, /* The assembled string with the logged
message */
+ *os, /* points to the os module */
+ *sep, /* line separator */
+ *tb_log; /* PyString with the assembled log msg */
+
+ hdr = PyString_FromString(Traceback (most recent call last):);
+ tmpl = PyString_FromString( File \%s\, line %s, in %s);
+ ftr = PyString_FromString();
+
+ tb_list = PyList_New(0); /* create the list of strings */
+ PyList_Append(tb_list, hdr); /* Append the header to the list */
+
+ /* 1st tb is useless; throw it away */
+ cur_tb = PyObject_GetAttrString(tb, tb_next);
+ while (cur_tb != Py_None)
+ {
+
+ ln = PyObject_GetAttrString(cur_tb, tb_lineno);
+ frame = PyObject_GetAttrString(cur_tb, tb_frame);
+ code = PyObject_GetAttrString(frame, f_code);
+ fn = PyObject_GetAttrString(code, co_filename);
+ nm = PyObject_GetAttrString(code, co_name);
+
+ args = Py_BuildValue((OOO), fn, ln, nm); /* args
tuple */
+ logline = PyString_Format(tmpl, args); /* build logged
string */
+ PyList_Append(tb_list, logline); /* append string
to list */
+
+ /* decrement references on all our objects */
+ Py_DECREF(logline);
+ Py_DECREF(args);
+ Py_XDECREF(nm);
+ Py_XDECREF(fn);
+ Py_XDECREF(code);
+ Py_XDECREF(frame);
+ Py_XDECREF(ln);
+
+ old_tb = cur_tb;
+ /* get the next traceback item */
+ cur_tb = PyObject_GetAttrString(cur_tb, tb_next);
+ Py_DECREF(old_tb);/* we're done with old_tb so
decref it */
+ }
+ PyList_Append(tb_list, ftr); /* append the log msg footer */
+
+ os = PyImport_ImportModule(os);
+ sep = PyObject_GetAttrString(os, linesep); /* get os EOL char
*/
+ tb_log = _PyString_Join(sep, tb_list); /* create tb log
msgs */
+ tbstr = PyString_AsString(tb_log);
+
+ Py_DECREF(tb_log);
+ Py_DECREF(sep);
+ Py_DECREF(os);
+ Py_DECREF(tb_list);
+ Py_DECREF(ftr);
+ Py_DECREF(tmpl);
+ Py_DECREF(hdr);
+ }
+ else
+ tbstr = No Traceback;
+
/*
* 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
@@ -2530,7 +2607,7 @@
* Assert() be more appropriate?
*/
estr = eob ? PyString_AsString(eob) :
