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
---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Reply via email to