Re: [HACKERS] pl/python tracebacks v2
On Wed, 2011-04-06 at 23:54 +0200, Jan Urbański wrote: Ouch, just today I found a flaw in this, namely that it assumes the lineno from the traceback always refers to the PL/Python function. If you create a PL/Python function that imports some code, runs it, and that code raises an exception, PLy_traceback will get utterly confused. Working on a fix... Here's the fix. Committed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks v2
On mån, 2011-03-21 at 00:40 +0100, Jan Urbański wrote: I finally got around to updating the PL/Python tracebacks patch. The other day I was writing some very simple PL/Python code and the lack of tracebacks is extremely annoying. I tweaked this a bit to make the patch less invasive, and then committed it. :) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks v2
On 06/04/11 21:38, Peter Eisentraut wrote: On mån, 2011-03-21 at 00:40 +0100, Jan Urbański wrote: I finally got around to updating the PL/Python tracebacks patch. The other day I was writing some very simple PL/Python code and the lack of tracebacks is extremely annoying. I tweaked this a bit to make the patch less invasive, and then committed it. :) Ouch, just today I found a flaw in this, namely that it assumes the lineno from the traceback always refers to the PL/Python function. If you create a PL/Python function that imports some code, runs it, and that code raises an exception, PLy_traceback will get utterly confused. Working on a fix... Jan PS: obviously it'd be great to have PL/Python traceback support in 9.1, but I sure hope we'll get some testing in beta for issues like this... J -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks v2
On 06/04/11 22:16, Jan Urbański wrote:
On 06/04/11 21:38, Peter Eisentraut wrote:
On mån, 2011-03-21 at 00:40 +0100, Jan Urbański wrote:
I finally got around to updating the PL/Python tracebacks patch. The
other day I was writing some very simple PL/Python code and the lack of
tracebacks is extremely annoying.
I tweaked this a bit to make the patch less invasive, and then committed
it. :)
Ouch, just today I found a flaw in this, namely that it assumes the
lineno from the traceback always refers to the PL/Python function. If
you create a PL/Python function that imports some code, runs it, and
that code raises an exception, PLy_traceback will get utterly confused.
Working on a fix...
Here's the fix.
The actual bug was funny. The traceback code was fetching the file line
from the traceback and trying to get that line from the original source
to print it. But sometimes that line was refering to a different source
file, like when the exception originated from an imported module.
In my testing I accidentally had the error (in a separate module) on
line 2, so the traceback code tried to fetch line 2 of the function,
which was completely whitespace. This can never happen in theory,
because you can't have a frame starting at an all-whitespace line. The
code to get that line was misbehaving and trying to do a malloc(-2),
which in turn was causing an ERROR invalid memory allocation.
All that is fixed with the attached patch.
Cheers,
Jan
PS: and thanks for committing that in the first place! :)
J
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 9352580..b2333b8 100644
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
*** get_source_line(const char *src, int lin
*** 4507,4512
--- 4507,4520
if (next == NULL)
return pstrdup(s);
+ /*
+ * Sanity check, next s if the line was all-whitespace, which should
+ * never happen if Python reported an frame created on that line, but
+ * check anyway.
+ */
+ if (next s)
+ return NULL;
+
return pnstrdup(s, next - s);
}
*** PLy_traceback(char **xmsg, char **tbmsg,
*** 4603,4608
--- 4611,4617
PyObject *volatile code = NULL;
PyObject *volatile name = NULL;
PyObject *volatile lineno = NULL;
+ PyObject *volatile filename = NULL;
PG_TRY();
{
*** PLy_traceback(char **xmsg, char **tbmsg,
*** 4621,4626
--- 4630,4639
name = PyObject_GetAttrString(code, co_name);
if (name == NULL)
elog(ERROR, could not get function name from Python code object);
+
+ filename = PyObject_GetAttrString(code, co_filename);
+ if (filename == NULL)
+ elog(ERROR, could not get file name from Python code object);
}
PG_CATCH();
{
*** PLy_traceback(char **xmsg, char **tbmsg,
*** 4628,4633
--- 4641,4647
Py_XDECREF(code);
Py_XDECREF(name);
Py_XDECREF(lineno);
+ Py_XDECREF(filename);
PG_RE_THROW();
}
PG_END_TRY();
*** PLy_traceback(char **xmsg, char **tbmsg,
*** 4638,4643
--- 4652,4658
char *proname;
char *fname;
char *line;
+ char *plain_filename;
long plain_lineno;
/*
*** PLy_traceback(char **xmsg, char **tbmsg,
*** 4651,4656
--- 4666,4672
fname = PyString_AsString(name);
proname = PLy_procedure_name(PLy_curr_procedure);
+ plain_filename = PyString_AsString(filename);
plain_lineno = PyInt_AsLong(lineno);
if (proname == NULL)
*** PLy_traceback(char **xmsg, char **tbmsg,
*** 4662,4668
tbstr, \n PL/Python function \%s\, line %ld, in %s,
proname, plain_lineno - 1, fname);
! if (PLy_curr_procedure)
{
/*
* If we know the current procedure, append the exact
--- 4678,4686
tbstr, \n PL/Python function \%s\, line %ld, in %s,
proname, plain_lineno - 1, fname);
! /* the code object was compiled with string as the filename */
! if (PLy_curr_procedure plain_filename != NULL
! strcmp(plain_filename, string) == 0)
{
/*
* If we know the current procedure, append the exact
*** PLy_traceback(char **xmsg, char **tbmsg,
*** 4670,4676
* traceback.py module behavior. We could store the
* already line-split source to avoid splitting it
* every time, but producing a traceback is not the
! * most important scenario to optimize for.
*/
line = get_source_line(PLy_curr_procedure-src, plain_lineno);
if (line)
--- 4688,4696
* traceback.py module behavior. We could store the
* already line-split source to avoid splitting it
* every time, but producing a traceback is not the
! * most important scenario to optimize for. However,
! * do not go as far as traceback.py in reading the source
! * of imported modules.
*/
line =
Re: [HACKERS] pl/python tracebacks v2
On 20 March 2011 23:40, Jan Urbański wulc...@wulczer.org wrote: I'll update the commitfest app for the 2011-Next commitfest, but if someone would like to pick this up and include it in the 9.1 PL/Python revamp pack, I'd be thrilled. I would also be thrilled. I definitely share your sense of frustration about the lack of tracebacks available when writing pl/python. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 07/03/11 22:55, Peter Eisentraut wrote: On mån, 2011-03-07 at 14:19 +0100, Jan Urbański wrote: On 07/03/11 14:01, Jan Urbański wrote: On 07/03/11 13:53, Peter Eisentraut wrote: On sön, 2011-03-06 at 13:14 +0100, Jan Urbański wrote: But fixing raise plpy.Fatal() to actually cause a FATAL is something that should be extracted from this patch and committed, even if the full patch does not make it. Um, what? I didn't find any details about this in this thread, nor a test case. So this in fact are three separate things, tracebacks, fix for plpy.Fatal and a one-line fix for reporting errors in Python iterators, that as I noticed has a side effect of changing the SQLCODE being raised :( I think I'll just respin the tracebacks patch as 3 separate ones, coming right up. Respun as three separate patches. Sorry for the confusion. BTW: looks like plpy.Fatal behaviour has been broken for quite some time now. Committed 1 and 2. Your traceback implementation in PLy_elog is now using two errdetail calls in one ereport call, which doesn't work (first one wins). Please reconsider that. Also, the comment still talks about the traceback going into detail. Gah, will look at this and fix. Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On ons, 2011-03-02 at 22:28 +0100, Jan Urbański wrote: I did some tests with the attached test script, calling various of the functions defined there and the error messages more or less made sense (or at least were not worse than before). Is that script part of the regression tests you added? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On sön, 2011-03-06 at 13:14 +0100, Jan Urbański wrote: But fixing raise plpy.Fatal() to actually cause a FATAL is something that should be extracted from this patch and committed, even if the full patch does not make it. Um, what? I didn't find any details about this in this thread, nor a test case. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 07/03/11 13:53, Peter Eisentraut wrote: On ons, 2011-03-02 at 22:28 +0100, Jan Urbański wrote: I did some tests with the attached test script, calling various of the functions defined there and the error messages more or less made sense (or at least were not worse than before). Is that script part of the regression tests you added? No, the regression tests are a bit different. Maybe this script should be part of them as well? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 07/03/11 13:53, Peter Eisentraut wrote: On sön, 2011-03-06 at 13:14 +0100, Jan Urbański wrote: But fixing raise plpy.Fatal() to actually cause a FATAL is something that should be extracted from this patch and committed, even if the full patch does not make it. Um, what? I didn't find any details about this in this thread, nor a test case. Yes, my fault for sneaking it here without more introduction than this comment several messages upthread: While testing I noticed that this broke raise plpy.Fatal() behaviour - it was no longer terminating the backend but just raising an error. That's fixed in this version. This patch also fixes a place where ereport is being used to report Python errors, which leads to losing the original error. Incidentally, this is exactly the issue that made diagnosing this bug: http://postgresql.1045698.n5.nabble.com/Bug-in-plpython-s-Python-Generators-td3230402.html so difficult. So this in fact are three separate things, tracebacks, fix for plpy.Fatal and a one-line fix for reporting errors in Python iterators, that as I noticed has a side effect of changing the SQLCODE being raised :( I think I'll just respin the tracebacks patch as 3 separate ones, coming right up. BTW, it's hard to test if raising plpy.Fatal actually causes a FATAL elog, because that would terminate the backend running the tests, and I though pg_regress treats this as an unconditional error (or am I mistaken?). Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 07/03/11 14:01, Jan Urbański wrote:
On 07/03/11 13:53, Peter Eisentraut wrote:
On sön, 2011-03-06 at 13:14 +0100, Jan Urbański wrote:
But fixing raise plpy.Fatal()
to actually cause a FATAL is something that should be extracted from
this patch and committed, even if the full patch does not make it.
Um, what? I didn't find any details about this in this thread, nor a
test case.
So this in fact are three separate things, tracebacks, fix for
plpy.Fatal and a one-line fix for reporting errors in Python iterators,
that as I noticed has a side effect of changing the SQLCODE being raised
:( I think I'll just respin the tracebacks patch as 3 separate ones,
coming right up.
Respun as three separate patches. Sorry for the confusion. BTW: looks
like plpy.Fatal behaviour has been broken for quite some time now.
Jan
From 06ac95d62de1aaf40dec020ac2892f20c3879db6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= wulc...@wulczer.org
Date: Mon, 7 Mar 2011 14:16:25 +0100
Subject: [PATCH 3/3] Add Python tracebacks to error messages.
For errors originating from Python exceptions add the traceback to the
message context. While at it rework the Python to Postgres error
passing mechanism a bit. A future optimisation might be not splitting
the procedure source each time a traceback is generated, but for now
it's probably not the most important scenario to optimise for.
---
src/pl/plpython/expected/plpython_do.out |5 +-
src/pl/plpython/expected/plpython_error.out| 193 +-
src/pl/plpython/expected/plpython_error_0.out | 193 +-
.../plpython/expected/plpython_subtransaction.out | 55 +++-
.../expected/plpython_subtransaction_0.out | 30 ++-
.../expected/plpython_subtransaction_5.out | 30 ++-
src/pl/plpython/expected/plpython_test.out |5 +-
src/pl/plpython/expected/plpython_types.out|5 +-
src/pl/plpython/expected/plpython_types_3.out |5 +-
src/pl/plpython/plpython.c | 287
src/pl/plpython/sql/plpython_error.sql | 105 +++
11 files changed, 821 insertions(+), 92 deletions(-)
diff --git a/src/pl/plpython/expected/plpython_do.out b/src/pl/plpython/expected/plpython_do.out
index a21b088..41b7a51 100644
--- a/src/pl/plpython/expected/plpython_do.out
+++ b/src/pl/plpython/expected/plpython_do.out
@@ -3,4 +3,7 @@ NOTICE: This is plpythonu.
CONTEXT: PL/Python anonymous code block
DO $$ nonsense $$ LANGUAGE plpythonu;
ERROR: NameError: global name 'nonsense' is not defined
-CONTEXT: PL/Python anonymous code block
+CONTEXT: Traceback (most recent call last):
+ PL/Python anonymous code block, line 1, in module
+nonsense
+PL/Python anonymous code block
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index e38ea60..0b7d87f 100644
--- a/src/pl/plpython/expected/plpython_error.out
+++ b/src/pl/plpython/expected/plpython_error.out
@@ -36,7 +36,10 @@ ERROR: spiexceptions.SyntaxError: syntax error at or near syntax
LINE 1: syntax error
^
QUERY: syntax error
-CONTEXT: PL/Python function sql_syntax_error
+CONTEXT: Traceback (most recent call last):
+ PL/Python function sql_syntax_error, line 1, in module
+plpy.execute(syntax error)
+PL/Python function sql_syntax_error
/* check the handling of uncaught python exceptions
*/
CREATE FUNCTION exception_index_invalid(text) RETURNS text
@@ -45,7 +48,10 @@ CREATE FUNCTION exception_index_invalid(text) RETURNS text
LANGUAGE plpythonu;
SELECT exception_index_invalid('test');
ERROR: IndexError: list index out of range
-CONTEXT: PL/Python function exception_index_invalid
+CONTEXT: Traceback (most recent call last):
+ PL/Python function exception_index_invalid, line 1, in module
+return args[1]
+PL/Python function exception_index_invalid
/* check handling of nested exceptions
*/
CREATE FUNCTION exception_index_invalid_nested() RETURNS text
@@ -59,7 +65,10 @@ LINE 1: SELECT test5('foo')
^
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
QUERY: SELECT test5('foo')
-CONTEXT: PL/Python function exception_index_invalid_nested
+CONTEXT: Traceback (most recent call last):
+ PL/Python function exception_index_invalid_nested, line 1, in module
+rv = plpy.execute(SELECT test5('foo'))
+PL/Python function exception_index_invalid_nested
/* a typo
*/
CREATE FUNCTION invalid_type_uncaught(a text) RETURNS text
@@ -75,7 +84,10 @@ return None
LANGUAGE plpythonu;
SELECT invalid_type_uncaught('rick');
ERROR: spiexceptions.UndefinedObject: type test does not exist
-CONTEXT: PL/Python function invalid_type_uncaught
+CONTEXT: Traceback (most recent call last):
+ PL/Python function invalid_type_uncaught, line 3, in module
+SD[plan] = plpy.prepare(q, [ test ])
+PL/Python function invalid_type_uncaught
/* for what it's worth
Re: [HACKERS] pl/python tracebacks
On mån, 2011-03-07 at 14:19 +0100, Jan Urbański wrote: On 07/03/11 14:01, Jan Urbański wrote: On 07/03/11 13:53, Peter Eisentraut wrote: On sön, 2011-03-06 at 13:14 +0100, Jan Urbański wrote: But fixing raise plpy.Fatal() to actually cause a FATAL is something that should be extracted from this patch and committed, even if the full patch does not make it. Um, what? I didn't find any details about this in this thread, nor a test case. So this in fact are three separate things, tracebacks, fix for plpy.Fatal and a one-line fix for reporting errors in Python iterators, that as I noticed has a side effect of changing the SQLCODE being raised :( I think I'll just respin the tracebacks patch as 3 separate ones, coming right up. Respun as three separate patches. Sorry for the confusion. BTW: looks like plpy.Fatal behaviour has been broken for quite some time now. Committed 1 and 2. Your traceback implementation in PLy_elog is now using two errdetail calls in one ereport call, which doesn't work (first one wins). Please reconsider that. Also, the comment still talks about the traceback going into detail. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 02/03/11 22:28, Jan Urbański wrote: On 01/03/11 22:12, Peter Eisentraut wrote: On tis, 2011-03-01 at 21:10 +0100, Jan Urbański wrote: So you end up with a context message saying PL/Python function %s and a detail message with the saved detail (if it's present) *and* the traceback. The problem is that the name of the function is already in the traceback, so there's no need for the context *if* there's a traceback present. I wouldn't actually worry about that bit of redundancy so much. Getting proper context for nested calls is much more important. Here's another version that puts tracebacks in the context field. I did some tests with the attached test script, calling various of the functions defined there and the error messages more or less made sense (or at least were not worse than before). I realized I did not update the patch state in the CF app when I added this version, so I flipped it back to Ready for Committer now. Tracebacks are a nice-to-have, so if we decide to drop this one due to time constraints, I'd understand that. But fixing raise plpy.Fatal() to actually cause a FATAL is something that should be extracted from this patch and committed, even if the full patch does not make it. Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 26/02/11 16:10, Peter Eisentraut wrote:
On lör, 2011-02-26 at 09:34 +0100, Jan Urbański wrote:
- Original message -
On Thu, Feb 24, 2011 at 9:03 AM, Jan Urbański wulc...@wulczer.org
wrote:
On 24/02/11 14:10, Peter Eisentraut wrote:
Hm, perhaps, I put it in the details, because it sounded like the place
to put information that is not that important, but still helpful. It's
kind of natural to think of the traceback as the detail of the error
message. But if you prefer context, I'm fine with that. You want me to
update the patch to put the traceback in the context?
I don't see a response to this question from Peter, but I read his
email to indicate that he was hoping you'd rework along these lines.
I can do that, but not until Monday evening.
Well, I was hoping for some other opinion, but I guess my request
stands.
I looked into putting the tracebacks in the context field, but IMHO it
doesn't really play out nice. PL/Python uses a errcontext callback to
populate the context field, so the reduntant information (the name of
the function) is always there. If that callback would be removed, the
context information will not appear in plpy.warning output, which I
think would be bad.
So: the context is currently put unconditionally into every elog
message, which I think is good. In case of errors, the traceback already
includes the procedure name (because of how Python tracebacks are
typically formatted), which makes the traceback contain redundant
information to the context field. Replacing the context field with the
traceback is difficult, because it is populated by the error context
callback.
After thinking about it more I believe that the context field should
keep on being a one line indication of which function the message comes
from (and that's how it's done in PL/pgSQL for instance), and the detail
field should be used for the details of the message, like the traceback
that comes with it, and that's what the attached patch does.
While testing I noticed that this broke raise plpy.Fatal() behaviour -
it was no longer terminating the backend but just raising an error.
That's fixed in this version. This patch also fixes a place where
ereport is being used to report Python errors, which leads to losing the
original error. Incidentally, this is exactly the issue that made
diagnosing this bug:
http://postgresql.1045698.n5.nabble.com/Bug-in-plpython-s-Python-Generators-td3230402.html
so difficult.
Cheers,
Jan
diff --git a/src/pl/plpython/expected/plpython_do.out b/src/pl/plpython/expected/plpython_do.out
index a21b088..fb0f0e5 100644
*** a/src/pl/plpython/expected/plpython_do.out
--- b/src/pl/plpython/expected/plpython_do.out
*** NOTICE: This is plpythonu.
*** 3,6
--- 3,9
CONTEXT: PL/Python anonymous code block
DO $$ nonsense $$ LANGUAGE plpythonu;
ERROR: NameError: global name 'nonsense' is not defined
+ DETAIL: Traceback (most recent call last):
+ PL/Python anonymous code block, line 1, in module
+ nonsense
CONTEXT: PL/Python anonymous code block
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index e38ea60..949c705 100644
*** a/src/pl/plpython/expected/plpython_error.out
--- b/src/pl/plpython/expected/plpython_error.out
*** SELECT sql_syntax_error();
*** 35,40
--- 35,43
ERROR: spiexceptions.SyntaxError: syntax error at or near syntax
LINE 1: syntax error
^
+ DETAIL: Traceback (most recent call last):
+ PL/Python function sql_syntax_error, line 1, in module
+ plpy.execute(syntax error)
QUERY: syntax error
CONTEXT: PL/Python function sql_syntax_error
/* check the handling of uncaught python exceptions
*** CREATE FUNCTION exception_index_invalid(
*** 45,50
--- 48,56
LANGUAGE plpythonu;
SELECT exception_index_invalid('test');
ERROR: IndexError: list index out of range
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid, line 1, in module
+ return args[1]
CONTEXT: PL/Python function exception_index_invalid
/* check handling of nested exceptions
*/
*** SELECT exception_index_invalid_nested();
*** 57,62
--- 63,71
ERROR: spiexceptions.UndefinedFunction: function test5(unknown) does not exist
LINE 1: SELECT test5('foo')
^
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid_nested, line 1, in module
+ rv = plpy.execute(SELECT test5('foo'))
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
QUERY: SELECT test5('foo')
CONTEXT: PL/Python function exception_index_invalid_nested
*** return None
*** 75,80
--- 84,92
LANGUAGE plpythonu;
SELECT invalid_type_uncaught('rick');
ERROR: spiexceptions.UndefinedObject: type test does not exist
+ DETAIL: Traceback (most recent call last):
+
Re: [HACKERS] pl/python tracebacks
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org writes: I looked into putting the tracebacks in the context field, but IMHO it doesn't really play out nice. PL/Python uses a errcontext callback to populate the context field, so the reduntant information (the name of the function) is always there. If that callback would be removed, the context information will not appear in plpy.warning output, which I think would be bad. So: the context is currently put unconditionally into every elog message, which I think is good. In case of errors, the traceback already includes the procedure name (because of how Python tracebacks are typically formatted), which makes the traceback contain redundant information to the context field. Replacing the context field with the traceback is difficult, because it is populated by the error context callback. After thinking about it more I believe that the context field should keep on being a one line indication of which function the message comes from (and that's how it's done in PL/pgSQL for instance), and the detail field should be used for the details of the message, like the traceback that comes with it, and that's what the attached patch does. To me, none of those arguments seem good. Traceback is the sort of thing that belongs in errcontext, and arbitarily deciding that plpython isn't going to play by the rules doesn't sit well here. I agree that what you are showing is redundant with the current errcontext printout, but the solution for that is to change the errcontext printout, not to add redundant and inappropriate errdetail. An example of the reasoning for this is the situation where a plpython function calls back into SQL, and something there throws an ereport (which might include an errdetail). It would be useful to include the Python traceback in the errcontext stack, since there might be multiple levels of Python function call within what PG sees as just a plpython function. But you can't get there with this approach. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 01/03/11 20:15, Tom Lane wrote: =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org writes: After thinking about it more I believe that the context field should keep on being a one line indication of which function the message comes from (and that's how it's done in PL/pgSQL for instance), and the detail field should be used for the details of the message, like the traceback that comes with it, and that's what the attached patch does. To me, none of those arguments seem good. Traceback is the sort of thing that belongs in errcontext, and arbitarily deciding that plpython isn't going to play by the rules doesn't sit well here. I agree that what you are showing is redundant with the current errcontext printout, but the solution for that is to change the errcontext printout, not to add redundant and inappropriate errdetail. An example of the reasoning for this is the situation where a plpython function calls back into SQL, and something there throws an ereport (which might include an errdetail). It would be useful to include the Python traceback in the errcontext stack, since there might be multiple levels of Python function call within what PG sees as just a plpython function. But you can't get there with this approach. Currently the traceback is added to the detail and the original errdetail is preserved. So you'd get the detail line and the traceback below it. But OK, since there are more voices in favour of putting tracebacks in the context field, I'll keep on looking for a solution. Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org writes: Currently the traceback is added to the detail and the original errdetail is preserved. So you'd get the detail line and the traceback below it. Hm? I'm talking about plpython_error_callback() and friends, which AFAICS you haven't changed the behavior of at all. And it would certainly be completely inappropriate to do what's said above for an errdetail with a non-plpython origin. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 01/03/11 20:35, Tom Lane wrote: =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org writes: Currently the traceback is added to the detail and the original errdetail is preserved. So you'd get the detail line and the traceback below it. Hm? I'm talking about plpython_error_callback() and friends, which AFAICS you haven't changed the behavior of at all. And it would certainly be completely inappropriate to do what's said above for an errdetail with a non-plpython origin. Not sure if I understand the problem. PL/Python sets plpython_error_callback right after entering the function call handler, so any elog thrown while the function is executing has a PL/Python function %s context message. If plpython calls into SQL with SPI and something there throws an elog(ERROR) with an errdetail, that detail is saved inside the exception and a Python error is then thrown. If this Python error reaches the top of the Python stack, the error reporting code kicks in, extracts the saved errdetail value from the Python exception, and then extract the stack trace and also adds it to the errdetail. So you end up with a context message saying PL/Python function %s and a detail message with the saved detail (if it's present) *and* the traceback. The problem is that the name of the function is already in the traceback, so there's no need for the context *if* there's a traceback present. The problem I'm having is technical: since the callback is already set when the code reaches the traceback-printing stage, you can't really unset it. AFAICS the elog code calls *all* callbacks from error_context_stack. So I can't prevent the context message from appearing. If I make the traceback part of the context as well, it's just going to appear together with the message from the callback. Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On Mar 1, 2011, at 12:10 PM, Jan Urbański wrote: So you end up with a context message saying PL/Python function %s and a detail message with the saved detail (if it's present) *and* the traceback. The problem is that the name of the function is already in the traceback, so there's no need for the context *if* there's a traceback present. The problem I'm having is technical: since the callback is already set when the code reaches the traceback-printing stage, you can't really unset it. AFAICS the elog code calls *all* callbacks from error_context_stack. So I can't prevent the context message from appearing. If I make the traceback part of the context as well, it's just going to appear together with the message from the callback. I remember going through a lot of pain getting this done right in pg-python[pl/py]. SELECT it_blows_up(); ERROR: function's main raised a Python exception CONTEXT: [exception from Python] Traceback (most recent call last): File public.it_blows_up(), line 13, in main three() File public.it_blows_up(), line 10, in three return two() File public.it_blows_up(), line 7, in two return one() File public.it_blows_up(), line 4, in one raise OverflowError(there's water everywhere) OverflowError: there's water everywhere [public.it_blows_up()] IIRC, I unconditionally print the [public.it_blows_up()] part iff it's not an ERROR. If it is an ERROR, I let the traceback rendering part of the code handle it on the PL's entry point exit. It was really tricky to do this because I was rendering the traceback *after* the error_context_stack had been called. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On tis, 2011-03-01 at 21:10 +0100, Jan Urbański wrote: So you end up with a context message saying PL/Python function %s and a detail message with the saved detail (if it's present) *and* the traceback. The problem is that the name of the function is already in the traceback, so there's no need for the context *if* there's a traceback present. I wouldn't actually worry about that bit of redundancy so much. Getting proper context for nested calls is much more important. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
- Original message - On Thu, Feb 24, 2011 at 9:03 AM, Jan Urbański wulc...@wulczer.org wrote: On 24/02/11 14:10, Peter Eisentraut wrote: Hm, perhaps, I put it in the details, because it sounded like the place to put information that is not that important, but still helpful. It's kind of natural to think of the traceback as the detail of the error message. But if you prefer context, I'm fine with that. You want me to update the patch to put the traceback in the context? I don't see a response to this question from Peter, but I read his email to indicate that he was hoping you'd rework along these lines. I can do that, but not until Monday evening. Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On lör, 2011-02-26 at 09:34 +0100, Jan Urbański wrote: - Original message - On Thu, Feb 24, 2011 at 9:03 AM, Jan Urbański wulc...@wulczer.org wrote: On 24/02/11 14:10, Peter Eisentraut wrote: Hm, perhaps, I put it in the details, because it sounded like the place to put information that is not that important, but still helpful. It's kind of natural to think of the traceback as the detail of the error message. But if you prefer context, I'm fine with that. You want me to update the patch to put the traceback in the context? I don't see a response to this question from Peter, but I read his email to indicate that he was hoping you'd rework along these lines. I can do that, but not until Monday evening. Well, I was hoping for some other opinion, but I guess my request stands. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On Thu, Feb 24, 2011 at 9:03 AM, Jan Urbański wulc...@wulczer.org wrote: On 24/02/11 14:10, Peter Eisentraut wrote: On tor, 2010-12-23 at 14:56 +0100, Jan Urbański wrote: For errors originating from Python exceptions add the traceback as the message detail. The patch tries to mimick Python's traceback.py module behaviour as close as possible, icluding interleaving stack frames with source code lines in the detail message. Any Python developer should instantly recognize these kind of error reporting, it looks almost the same as an error in the interactive Python shell. I think the traceback should go into the CONTEXT part of the error. The context message that's already there is now redundant with the traceback. You could even call errcontext() multiple times to build up the traceback, but maybe that's not necessary. Hm, perhaps, I put it in the details, because it sounded like the place to put information that is not that important, but still helpful. It's kind of natural to think of the traceback as the detail of the error message. But if you prefer context, I'm fine with that. You want me to update the patch to put the traceback in the context? I don't see a response to this question from Peter, but I read his email to indicate that he was hoping you'd rework along these lines. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On lör, 2011-02-12 at 02:00 -0700, Alex Hunsaker wrote: PyString_AsString is used all over the place without any pfrees. But I have no Idea how that pstrdup() is getting freed if at all. pstrdup() like palloc() allocates memory from the current memory context, which is freed automatically at some useful time, often at the end of the query. It is very common throughout the PostgreSQL code that memory is not explicitly freed. See src/backend/utils/mmgr/README for more information. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On lör, 2011-02-12 at 10:07 +0100, Jan Urbański wrote:
PLyUnicode_AsString(PyObject *unicode)
{
PyObject *o = PLyUnicode_Bytes(unicode);
char *rv = pstrdup(PyBytes_AsString(o));
Py_XDECREF(o);
return rv;
}
PyString_AsString is used all over the place without any pfrees. But
I
have no Idea how that pstrdup() is getting freed if at all.
Care to enlighten me ?
Ooops, seems that this hack that's meant to improve compatibility with
Python3 makes it leak. I wonder is the pstrdup is necessary here,
The result of PyBytes_AsString(o) points into the internal storage of o,
which is released (effectively freed) by the decref on the next line.
So you'd better make a copy if you want to keep using it.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On tor, 2010-12-23 at 14:56 +0100, Jan Urbański wrote: For errors originating from Python exceptions add the traceback as the message detail. The patch tries to mimick Python's traceback.py module behaviour as close as possible, icluding interleaving stack frames with source code lines in the detail message. Any Python developer should instantly recognize these kind of error reporting, it looks almost the same as an error in the interactive Python shell. I think the traceback should go into the CONTEXT part of the error. The context message that's already there is now redundant with the traceback. You could even call errcontext() multiple times to build up the traceback, but maybe that's not necessary. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 24/02/11 14:10, Peter Eisentraut wrote: On tor, 2010-12-23 at 14:56 +0100, Jan Urbański wrote: For errors originating from Python exceptions add the traceback as the message detail. The patch tries to mimick Python's traceback.py module behaviour as close as possible, icluding interleaving stack frames with source code lines in the detail message. Any Python developer should instantly recognize these kind of error reporting, it looks almost the same as an error in the interactive Python shell. I think the traceback should go into the CONTEXT part of the error. The context message that's already there is now redundant with the traceback. You could even call errcontext() multiple times to build up the traceback, but maybe that's not necessary. Hm, perhaps, I put it in the details, because it sounded like the place to put information that is not that important, but still helpful. It's kind of natural to think of the traceback as the detail of the error message. But if you prefer context, I'm fine with that. You want me to update the patch to put the traceback in the context? Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 12/02/11 04:12, Alex Hunsaker wrote: On Wed, Feb 9, 2011 at 02:10, Jan Urbański wulc...@wulczer.org wrote: On 06/02/11 20:12, Jan Urbański wrote: On 27/01/11 22:58, Jan Urbański wrote: On 23/12/10 14:56, Jan Urbański wrote: Here's a patch implementing traceback support for PL/Python mentioned in http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's an incremental patch on top of the plpython-refactor patch sent eariler. Updated to master. Updated to master again. Once more. In PLy_traceback fname and prname look like they will leak (well as much as a palloc() in an error path can leak I suppose). But they're no palloc'd, no? fname is either a static module string, or PyString_AsString, which also doesn't allocate memory, AFAIK. proname is also a static string. They're transferred to heap-allocated memory in appendStringInfo, which gets pfreed after emitting the error message. Marking as Ready. Thanks! Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On Sat, Feb 12, 2011 at 01:50, Jan Urbański wulc...@wulczer.org wrote:
On 12/02/11 04:12, Alex Hunsaker wrote:
In PLy_traceback fname and prname look like they will leak (well as
much as a palloc() in an error path can leak I suppose).
But they're no palloc'd, no? fname is either a static module string,
or PyString_AsString, which also doesn't allocate memory, AFAIK.
Yeah, I was flat out wrong about proname :-(.
As for fname, I must be missing some magic. We have:
#if PY_MAJOR_VERSION 3
...
#define PyString_AsString(x) PLyUnicode_AsString(x)
PLyUnicode_AsString(PyObject *unicode)
{
PyObject *o = PLyUnicode_Bytes(unicode);
char *rv = pstrdup(PyBytes_AsString(o));
Py_XDECREF(o);
return rv;
}
PyString_AsString is used all over the place without any pfrees. But I
have no Idea how that pstrdup() is getting freed if at all.
Care to enlighten me ?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 12/02/11 10:00, Alex Hunsaker wrote:
On Sat, Feb 12, 2011 at 01:50, Jan Urbański wulc...@wulczer.org wrote:
On 12/02/11 04:12, Alex Hunsaker wrote:
In PLy_traceback fname and prname look like they will leak (well as
much as a palloc() in an error path can leak I suppose).
But they're no palloc'd, no? fname is either a static module string,
or PyString_AsString, which also doesn't allocate memory, AFAIK.
Yeah, I was flat out wrong about proname :-(.
As for fname, I must be missing some magic. We have:
#if PY_MAJOR_VERSION 3
...
#define PyString_AsString(x) PLyUnicode_AsString(x)
PLyUnicode_AsString(PyObject *unicode)
{
PyObject *o = PLyUnicode_Bytes(unicode);
char *rv = pstrdup(PyBytes_AsString(o));
Py_XDECREF(o);
return rv;
}
PyString_AsString is used all over the place without any pfrees. But I
have no Idea how that pstrdup() is getting freed if at all.
Care to enlighten me ?
Ooops, seems that this hack that's meant to improve compatibility with
Python3 makes it leak. I wonder is the pstrdup is necessary here, but
OTOH the leak should not be overly significant, given that no-one
complained about it before... and PyString_AsString is being used in
several other places.
Jan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On Wed, Feb 9, 2011 at 4:10 AM, Jan Urbański wulc...@wulczer.org wrote: On 06/02/11 20:12, Jan Urbański wrote: On 27/01/11 22:58, Jan Urbański wrote: On 23/12/10 14:56, Jan Urbański wrote: Here's a patch implementing traceback support for PL/Python mentioned in http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's an incremental patch on top of the plpython-refactor patch sent eariler. Updated to master. Updated to master again. Once more. Alex Hunsaker is listed as the reviewer for this patch, but I don't see a review posted. If this feature is still wanted for 9.1, can someone jump in here? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On Fri, Feb 11, 2011 at 08:45, Robert Haas robertmh...@gmail.com wrote: On Wed, Feb 9, 2011 at 4:10 AM, Jan Urbański wulc...@wulczer.org wrote: On 06/02/11 20:12, Jan Urbański wrote: On 27/01/11 22:58, Jan Urbański wrote: On 23/12/10 14:56, Jan Urbański wrote: Here's a patch implementing traceback support for PL/Python mentioned in http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's an incremental patch on top of the plpython-refactor patch sent eariler. Updated to master. Updated to master again. Once more. Alex Hunsaker is listed as the reviewer for this patch, but I don't see a review posted. If this feature is still wanted for 9.1, can someone jump in here? Goodness... I picked up this patch the day before yesterday because no-one was listed. That being said, if anyone else wants to beat me to the punch of reviewing this, have at it! The more eyes the merrier! I wish I could squeeze the lime of my time to find a few more hours -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On Fri, Feb 11, 2011 at 11:22 AM, Alex Hunsaker bada...@gmail.com wrote: On Fri, Feb 11, 2011 at 08:45, Robert Haas robertmh...@gmail.com wrote: On Wed, Feb 9, 2011 at 4:10 AM, Jan Urbański wulc...@wulczer.org wrote: On 06/02/11 20:12, Jan Urbański wrote: On 27/01/11 22:58, Jan Urbański wrote: On 23/12/10 14:56, Jan Urbański wrote: Here's a patch implementing traceback support for PL/Python mentioned in http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's an incremental patch on top of the plpython-refactor patch sent eariler. Updated to master. Updated to master again. Once more. Alex Hunsaker is listed as the reviewer for this patch, but I don't see a review posted. If this feature is still wanted for 9.1, can someone jump in here? Goodness... I picked up this patch the day before yesterday because no-one was listed. That being said, if anyone else wants to beat me to the punch of reviewing this, have at it! The more eyes the merrier! Sorry, I didn't see when you'd picked it up. I was just keeping an eye on my wall calendar. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
Robert Haas robertmh...@gmail.com wrote: Goodness... I picked up this patch the day before yesterday because no-one was listed. That being said, if anyone else wants to beat me to the punch of reviewing this, have at it! The more eyes the merrier! Sorry, I didn't see when you'd picked it up. I was just keeping an eye on my wall calendar. [OT] FWIW, this is the sort of situation which caused me to suggest that the web app somehow show the date of the last reviewer change when it is past the Last Activity date. I don't really care whether it would be in the Reviewers column or as a second line, in parentheses, in the Last Activity column. I would find it useful when managing a CF, anyway -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On Wed, Feb 9, 2011 at 02:10, Jan Urbański wulc...@wulczer.org wrote: On 06/02/11 20:12, Jan Urbański wrote: On 27/01/11 22:58, Jan Urbański wrote: On 23/12/10 14:56, Jan Urbański wrote: Here's a patch implementing traceback support for PL/Python mentioned in http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's an incremental patch on top of the plpython-refactor patch sent eariler. Updated to master. Updated to master again. Once more. In PLy_traceback fname and prname look like they will leak (well as much as a palloc() in an error path can leak I suppose). Other than that everything looks good. I tested plpython2 and plpython3 and skimmed the docs to see if there was anything obvious that needed updating. I also obviously looked at the added regression tests and made sure they worked. Marking as Ready. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python tracebacks
On 06/02/11 20:12, Jan Urbański wrote:
On 27/01/11 22:58, Jan Urbański wrote:
On 23/12/10 14:56, Jan Urbański wrote:
Here's a patch implementing traceback support for PL/Python mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.
Updated to master.
Updated to master again.
Once more.
diff --git a/src/pl/plpython/expected/plpython_do.out b/src/pl/plpython/expected/plpython_do.out
index a21b088..fb0f0e5 100644
*** a/src/pl/plpython/expected/plpython_do.out
--- b/src/pl/plpython/expected/plpython_do.out
*** NOTICE: This is plpythonu.
*** 3,6
--- 3,9
CONTEXT: PL/Python anonymous code block
DO $$ nonsense $$ LANGUAGE plpythonu;
ERROR: NameError: global name 'nonsense' is not defined
+ DETAIL: Traceback (most recent call last):
+ PL/Python anonymous code block, line 1, in module
+ nonsense
CONTEXT: PL/Python anonymous code block
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index 7597ca7..08b6ba4 100644
*** a/src/pl/plpython/expected/plpython_error.out
--- b/src/pl/plpython/expected/plpython_error.out
*** SELECT sql_syntax_error();
*** 35,40
--- 35,43
ERROR: plpy.SPIError: syntax error at or near syntax
LINE 1: syntax error
^
+ DETAIL: Traceback (most recent call last):
+ PL/Python function sql_syntax_error, line 1, in module
+ plpy.execute(syntax error)
QUERY: syntax error
CONTEXT: PL/Python function sql_syntax_error
/* check the handling of uncaught python exceptions
*** CREATE FUNCTION exception_index_invalid(
*** 45,50
--- 48,56
LANGUAGE plpythonu;
SELECT exception_index_invalid('test');
ERROR: IndexError: list index out of range
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid, line 1, in module
+ return args[1]
CONTEXT: PL/Python function exception_index_invalid
/* check handling of nested exceptions
*/
*** SELECT exception_index_invalid_nested();
*** 57,62
--- 63,71
ERROR: plpy.SPIError: function test5(unknown) does not exist
LINE 1: SELECT test5('foo')
^
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid_nested, line 1, in module
+ rv = plpy.execute(SELECT test5('foo'))
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
QUERY: SELECT test5('foo')
CONTEXT: PL/Python function exception_index_invalid_nested
*** return None
*** 75,80
--- 84,92
LANGUAGE plpythonu;
SELECT invalid_type_uncaught('rick');
ERROR: plpy.SPIError: type test does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function invalid_type_uncaught, line 3, in module
+ SD[plan] = plpy.prepare(q, [ test ])
CONTEXT: PL/Python function invalid_type_uncaught
/* for what it's worth catch the exception generated by
* the typo, and return None
*** return None
*** 121,126
--- 133,141
LANGUAGE plpythonu;
SELECT invalid_type_reraised('rick');
ERROR: plpy.Error: type test does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function invalid_type_reraised, line 6, in module
+ plpy.error(str(ex))
CONTEXT: PL/Python function invalid_type_reraised
/* no typo no messing about
*/
*** SELECT valid_type('rick');
*** 140,145
--- 155,255
(1 row)
+ /* error in nested functions to get a traceback
+ */
+ CREATE FUNCTION nested_error() RETURNS text
+ AS
+ 'def fun1():
+ plpy.error(boom)
+
+ def fun2():
+ fun1()
+
+ def fun3():
+ fun2()
+
+ fun3()
+ return not reached
+ '
+ LANGUAGE plpythonu;
+ SELECT nested_error();
+ ERROR: plpy.Error: boom
+ DETAIL: Traceback (most recent call last):
+ PL/Python function nested_error, line 10, in module
+ fun3()
+ PL/Python function nested_error, line 8, in fun3
+ fun2()
+ PL/Python function nested_error, line 5, in fun2
+ fun1()
+ PL/Python function nested_error, line 2, in fun1
+ plpy.error(boom)
+ CONTEXT: PL/Python function nested_error
+ /* raising plpy.Error is just like calling plpy.error
+ */
+ CREATE FUNCTION nested_error_raise() RETURNS text
+ AS
+ 'def fun1():
+ raise plpy.Error(boom)
+
+ def fun2():
+ fun1()
+
+ def fun3():
+ fun2()
+
+ fun3()
+ return not reached
+ '
+ LANGUAGE plpythonu;
+ SELECT nested_error_raise();
+ ERROR: plpy.Error: boom
+ DETAIL: Traceback (most recent call last):
+ PL/Python function nested_error_raise, line 10, in module
+ fun3()
+ PL/Python function nested_error_raise, line 8, in fun3
+ fun2()
+ PL/Python function nested_error_raise, line 5, in fun2
+ fun1()
+ PL/Python function nested_error_raise, line 2, in fun1
+ raise plpy.Error(boom)
+ CONTEXT: PL/Python function
Re: [HACKERS] pl/python tracebacks
On 27/01/11 22:58, Jan Urbański wrote:
On 23/12/10 14:56, Jan Urbański wrote:
Here's a patch implementing traceback support for PL/Python mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.
Updated to master.
Updated to master again.
diff --git a/src/pl/plpython/expected/plpython_do.out b/src/pl/plpython/expected/plpython_do.out
index a21b088..fb0f0e5 100644
*** a/src/pl/plpython/expected/plpython_do.out
--- b/src/pl/plpython/expected/plpython_do.out
*** NOTICE: This is plpythonu.
*** 3,6
--- 3,9
CONTEXT: PL/Python anonymous code block
DO $$ nonsense $$ LANGUAGE plpythonu;
ERROR: NameError: global name 'nonsense' is not defined
+ DETAIL: Traceback (most recent call last):
+ PL/Python anonymous code block, line 1, in module
+ nonsense
CONTEXT: PL/Python anonymous code block
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index 7597ca7..08b6ba4 100644
*** a/src/pl/plpython/expected/plpython_error.out
--- b/src/pl/plpython/expected/plpython_error.out
*** SELECT sql_syntax_error();
*** 35,40
--- 35,43
ERROR: plpy.SPIError: syntax error at or near syntax
LINE 1: syntax error
^
+ DETAIL: Traceback (most recent call last):
+ PL/Python function sql_syntax_error, line 1, in module
+ plpy.execute(syntax error)
QUERY: syntax error
CONTEXT: PL/Python function sql_syntax_error
/* check the handling of uncaught python exceptions
*** CREATE FUNCTION exception_index_invalid(
*** 45,50
--- 48,56
LANGUAGE plpythonu;
SELECT exception_index_invalid('test');
ERROR: IndexError: list index out of range
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid, line 1, in module
+ return args[1]
CONTEXT: PL/Python function exception_index_invalid
/* check handling of nested exceptions
*/
*** SELECT exception_index_invalid_nested();
*** 57,62
--- 63,71
ERROR: plpy.SPIError: function test5(unknown) does not exist
LINE 1: SELECT test5('foo')
^
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid_nested, line 1, in module
+ rv = plpy.execute(SELECT test5('foo'))
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
QUERY: SELECT test5('foo')
CONTEXT: PL/Python function exception_index_invalid_nested
*** return None
*** 75,80
--- 84,92
LANGUAGE plpythonu;
SELECT invalid_type_uncaught('rick');
ERROR: plpy.SPIError: type test does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function invalid_type_uncaught, line 3, in module
+ SD[plan] = plpy.prepare(q, [ test ])
CONTEXT: PL/Python function invalid_type_uncaught
/* for what it's worth catch the exception generated by
* the typo, and return None
*** return None
*** 121,126
--- 133,141
LANGUAGE plpythonu;
SELECT invalid_type_reraised('rick');
ERROR: plpy.Error: type test does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function invalid_type_reraised, line 6, in module
+ plpy.error(str(ex))
CONTEXT: PL/Python function invalid_type_reraised
/* no typo no messing about
*/
*** SELECT valid_type('rick');
*** 140,145
--- 155,255
(1 row)
+ /* error in nested functions to get a traceback
+ */
+ CREATE FUNCTION nested_error() RETURNS text
+ AS
+ 'def fun1():
+ plpy.error(boom)
+
+ def fun2():
+ fun1()
+
+ def fun3():
+ fun2()
+
+ fun3()
+ return not reached
+ '
+ LANGUAGE plpythonu;
+ SELECT nested_error();
+ ERROR: plpy.Error: boom
+ DETAIL: Traceback (most recent call last):
+ PL/Python function nested_error, line 10, in module
+ fun3()
+ PL/Python function nested_error, line 8, in fun3
+ fun2()
+ PL/Python function nested_error, line 5, in fun2
+ fun1()
+ PL/Python function nested_error, line 2, in fun1
+ plpy.error(boom)
+ CONTEXT: PL/Python function nested_error
+ /* raising plpy.Error is just like calling plpy.error
+ */
+ CREATE FUNCTION nested_error_raise() RETURNS text
+ AS
+ 'def fun1():
+ raise plpy.Error(boom)
+
+ def fun2():
+ fun1()
+
+ def fun3():
+ fun2()
+
+ fun3()
+ return not reached
+ '
+ LANGUAGE plpythonu;
+ SELECT nested_error_raise();
+ ERROR: plpy.Error: boom
+ DETAIL: Traceback (most recent call last):
+ PL/Python function nested_error_raise, line 10, in module
+ fun3()
+ PL/Python function nested_error_raise, line 8, in fun3
+ fun2()
+ PL/Python function nested_error_raise, line 5, in fun2
+ fun1()
+ PL/Python function nested_error_raise, line 2, in fun1
+ raise plpy.Error(boom)
+ CONTEXT: PL/Python function nested_error_raise
+ /* using plpy.warning should not
Re: [HACKERS] pl/python tracebacks
On 23/12/10 14:56, Jan Urbański wrote:
Here's a patch implementing traceback support for PL/Python mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.
Updated to master.
diff --git a/src/pl/plpython/expected/plpython_do.out b/src/pl/plpython/expected/plpython_do.out
index a21b088..fb0f0e5 100644
*** a/src/pl/plpython/expected/plpython_do.out
--- b/src/pl/plpython/expected/plpython_do.out
*** NOTICE: This is plpythonu.
*** 3,6
--- 3,9
CONTEXT: PL/Python anonymous code block
DO $$ nonsense $$ LANGUAGE plpythonu;
ERROR: NameError: global name 'nonsense' is not defined
+ DETAIL: Traceback (most recent call last):
+ PL/Python anonymous code block, line 1, in module
+ nonsense
CONTEXT: PL/Python anonymous code block
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index ea4a54c..1e6295e 100644
*** a/src/pl/plpython/expected/plpython_error.out
--- b/src/pl/plpython/expected/plpython_error.out
*** CONTEXT: PL/Python function sql_syntax
*** 13,18
--- 13,21
ERROR: plpy.SPIError: syntax error at or near syntax
LINE 1: syntax error
^
+ DETAIL: Traceback (most recent call last):
+ PL/Python function sql_syntax_error, line 1, in module
+ plpy.execute(syntax error)
QUERY: syntax error
CONTEXT: PL/Python function sql_syntax_error
/* check the handling of uncaught python exceptions
*** CREATE FUNCTION exception_index_invalid(
*** 23,28
--- 26,34
LANGUAGE plpythonu;
SELECT exception_index_invalid('test');
ERROR: IndexError: list index out of range
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid, line 1, in module
+ return args[1]
CONTEXT: PL/Python function exception_index_invalid
/* check handling of nested exceptions
*/
*** CONTEXT: PL/Python function exception_
*** 37,42
--- 43,51
ERROR: plpy.SPIError: function test5(unknown) does not exist
LINE 1: SELECT test5('foo')
^
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid_nested, line 1, in module
+ rv = plpy.execute(SELECT test5('foo'))
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
QUERY: SELECT test5('foo')
CONTEXT: PL/Python function exception_index_invalid_nested
*** SELECT invalid_type_uncaught('rick');
*** 57,62
--- 66,74
WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function invalid_type_uncaught
ERROR: plpy.SPIError: type test does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function invalid_type_uncaught, line 3, in module
+ SD[plan] = plpy.prepare(q, [ test ])
CONTEXT: PL/Python function invalid_type_uncaught
/* for what it's worth catch the exception generated by
* the typo, and return None
*** SELECT invalid_type_reraised('rick');
*** 107,112
--- 119,127
WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function invalid_type_reraised
ERROR: plpy.Error: type test does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function invalid_type_reraised, line 6, in module
+ plpy.error(str(ex))
CONTEXT: PL/Python function invalid_type_reraised
/* no typo no messing about
*/
*** SELECT valid_type('rick');
*** 126,128
--- 141,238
(1 row)
+ /* error in nested functions to get a traceback
+ */
+ CREATE FUNCTION nested_error() RETURNS text
+ AS
+ 'def fun1():
+ plpy.error(boom)
+
+ def fun2():
+ fun1()
+
+ def fun3():
+ fun2()
+
+ fun3()
+ return not reached
+ '
+ LANGUAGE plpythonu;
+ SELECT nested_error();
+ ERROR: plpy.Error: boom
+ DETAIL: Traceback (most recent call last):
+ PL/Python function nested_error, line 10, in module
+ fun3()
+ PL/Python function nested_error, line 8, in fun3
+ fun2()
+ PL/Python function nested_error, line 5, in fun2
+ fun1()
+ PL/Python function nested_error, line 2, in fun1
+ plpy.error(boom)
+ CONTEXT: PL/Python function nested_error
+ /* raising plpy.Error is just like calling plpy.error
+ */
+ CREATE FUNCTION nested_error_raise() RETURNS text
+ AS
+ 'def fun1():
+ raise plpy.Error(boom)
+
+ def fun2():
+ fun1()
+
+ def fun3():
+ fun2()
+
+ fun3()
+ return not reached
+ '
+ LANGUAGE plpythonu;
+ SELECT nested_error_raise();
+ ERROR: plpy.Error: boom
+ DETAIL: Traceback (most recent call last):
+ PL/Python function nested_error_raise, line 10, in module
+ fun3()
+ PL/Python function nested_error_raise, line 8, in fun3
+ fun2()
+ PL/Python function nested_error_raise, line 5, in fun2
+ fun1()
+ PL/Python function nested_error_raise, line 2, in fun1
+
[HACKERS] pl/python tracebacks
Here's a patch implementing traceback support for PL/Python mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.
Git branch for this patch:
https://github.com/wulczer/postgres/tree/tracebacks.
It's a variant of
http://archives.postgresql.org/pgsql-patches/2006-02/msg00288.php with a
few more twists.
For errors originating from Python exceptions add the traceback as the
message detail. The patch tries to mimick Python's traceback.py module
behaviour as close as possible, icluding interleaving stack frames with
source code lines in the detail message. Any Python developer should
instantly recognize these kind of error reporting, it looks almost the
same as an error in the interactive Python shell.
A future optimisation might be not splitting the procedure source each
time a traceback is generated, but for now it's probably not the most
important scenario to optimise for.
Cheers,
Jan
diff --git a/src/pl/plpython/expected/plpython_do.out b/src/pl/plpython/expected/plpython_do.out
index a21b088..fb0f0e5 100644
*** a/src/pl/plpython/expected/plpython_do.out
--- b/src/pl/plpython/expected/plpython_do.out
*** NOTICE: This is plpythonu.
*** 3,6
--- 3,9
CONTEXT: PL/Python anonymous code block
DO $$ nonsense $$ LANGUAGE plpythonu;
ERROR: NameError: global name 'nonsense' is not defined
+ DETAIL: Traceback (most recent call last):
+ PL/Python anonymous code block, line 1, in module
+ nonsense
CONTEXT: PL/Python anonymous code block
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index 70890a8..fe8a91f 100644
*** a/src/pl/plpython/expected/plpython_error.out
--- b/src/pl/plpython/expected/plpython_error.out
*** SELECT sql_syntax_error();
*** 11,16
--- 11,19
WARNING: plpy.SPIError: unrecognized error in PLy_spi_execute_query
CONTEXT: PL/Python function sql_syntax_error
ERROR: plpy.SPIError: syntax error at or near syntax
+ DETAIL: Traceback (most recent call last):
+ PL/Python function sql_syntax_error, line 1, in module
+ plpy.execute(syntax error)
CONTEXT: PL/Python function sql_syntax_error
/* check the handling of uncaught python exceptions
*/
*** CREATE FUNCTION exception_index_invalid(
*** 20,25
--- 23,31
LANGUAGE plpythonu;
SELECT exception_index_invalid('test');
ERROR: IndexError: list index out of range
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid, line 1, in module
+ return args[1]
CONTEXT: PL/Python function exception_index_invalid
/* check handling of nested exceptions
*/
*** SELECT exception_index_invalid_nested();
*** 32,37
--- 38,46
WARNING: plpy.SPIError: unrecognized error in PLy_spi_execute_query
CONTEXT: PL/Python function exception_index_invalid_nested
ERROR: plpy.SPIError: function test5(unknown) does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function exception_index_invalid_nested, line 1, in module
+ rv = plpy.execute(SELECT test5('foo'))
CONTEXT: PL/Python function exception_index_invalid_nested
/* a typo
*/
*** SELECT invalid_type_uncaught('rick');
*** 50,55
--- 59,67
WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function invalid_type_uncaught
ERROR: plpy.SPIError: type test does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function invalid_type_uncaught, line 3, in module
+ SD[plan] = plpy.prepare(q, [ test ])
CONTEXT: PL/Python function invalid_type_uncaught
/* for what it's worth catch the exception generated by
* the typo, and return None
*** SELECT invalid_type_reraised('rick');
*** 100,105
--- 112,120
WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function invalid_type_reraised
ERROR: plpy.Error: type test does not exist
+ DETAIL: Traceback (most recent call last):
+ PL/Python function invalid_type_reraised, line 6, in module
+ plpy.error(str(ex))
CONTEXT: PL/Python function invalid_type_reraised
/* no typo no messing about
*/
*** SELECT valid_type('rick');
*** 119,121
--- 134,231
(1 row)
+ /* error in nested functions to get a traceback
+ */
+ CREATE FUNCTION nested_error() RETURNS text
+ AS
+ 'def fun1():
+ plpy.error(boom)
+
+ def fun2():
+ fun1()
+
+ def fun3():
+ fun2()
+
+ fun3()
+ return not reached
+ '
+ LANGUAGE plpythonu;
+ SELECT nested_error();
+ ERROR: plpy.Error: boom
+ DETAIL: Traceback (most recent call last):
+ PL/Python function nested_error, line 10, in module
+ fun3()
+ PL/Python function nested_error, line 8, in fun3
+ fun2()
+ PL/Python function nested_error, line 5, in fun2
+ fun1()
+ PL/Python function nested_error,
