pgsql: Add traceback information to PL/Python errors

On Wed, Apr 6, 2011 at 3:37 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Add traceback information to PL/Python errors

This mimics the traceback information the Python interpreter prints
with exceptions.

Jan Urbański

On my system this spits out a warning:

plpython.c: In function ‘PLy_traceback’:
plpython.c:4487: warning: ‘s’ may be used uninitialized in this function
plpython.c:4487: note: ‘s’ was declared here

That appears to be a live bug, unless it's guaranteed that lineno will
always be > 0.

Also, the loop test should really be written as current < lineno,
rather than current != lineno, just in case we should manage to pass a
lineno < 0, which with the current code would go into the tank and
spin.

This part looks pretty sketchy, too:

while (s && isspace((unsigned char) *s))
s++;

Perhaps we meant to test *s here. It's hard to believe that we're
really intending to test whether the pointer has fallen off the end of
the address space and wrapped around to NULL.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 07/04/11 23:01, Robert Haas wrote:

On Wed, Apr 6, 2011 at 3:37 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Add traceback information to PL/Python errors

This mimics the traceback information the Python interpreter prints
with exceptions.

Jan Urbański

On my system this spits out a warning:

plpython.c: In function ‘PLy_traceback’:
plpython.c:4487: warning: ‘s’ may be used uninitialized in this function
plpython.c:4487: note: ‘s’ was declared here

That appears to be a live bug, unless it's guaranteed that lineno will
always be > 0.

lineno should be > 0, unless Python is trying to tell us that the code
frame originates from before the function.

Also, the loop test should really be written as current < lineno,
rather than current != lineno, just in case we should manage to pass a
lineno < 0, which with the current code would go into the tank and
spin.

Yeah, good point.

This part looks pretty sketchy, too:

while (s && isspace((unsigned char) *s))
s++;

Perhaps we meant to test *s here. It's hard to believe that we're
really intending to test whether the pointer has fallen off the end of
the address space and wrapped around to NULL.

Gah, so short a function and so many things that I managed to get wrong.

There's also this:
http://archives.postgresql.org/pgsql-hackers/2011-04/msg00334.php

Jan

On Thu, Apr 7, 2011 at 5:06 PM, Jan Urbański <wulczer@wulczer.org> wrote:

On 07/04/11 23:01, Robert Haas wrote:

On Wed, Apr 6, 2011 at 3:37 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Add traceback information to PL/Python errors

This mimics the traceback information the Python interpreter prints
with exceptions.

Jan Urbański

On my system this spits out a warning:

plpython.c: In function ‘PLy_traceback’:
plpython.c:4487: warning: ‘s’ may be used uninitialized in this function
plpython.c:4487: note: ‘s’ was declared here

That appears to be a live bug, unless it's guaranteed that lineno will
always be > 0.

lineno should be > 0, unless Python is trying to tell us that the code
frame originates from before the function.

Also, the loop test should really be written as current < lineno,
rather than current != lineno, just in case we should manage to pass a
lineno < 0, which with the current code would go into the tank and
spin.

Yeah, good point.

This part looks pretty sketchy, too:

    while (s && isspace((unsigned char) *s))
        s++;

Perhaps we meant to test *s here.  It's hard to believe that we're
really intending to test whether the pointer has fallen off the end of
the address space and wrapped around to NULL.

Gah, so short a function and so many things that I managed to get wrong.

Patch?

There's also this:
http://archives.postgresql.org/pgsql-hackers/2011-04/msg00334.php

Yep. I am assuming Peter will look at that one.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 08/04/11 00:25, Robert Haas wrote:

On Thu, Apr 7, 2011 at 5:06 PM, Jan Urbański <wulczer@wulczer.org> wrote:

On 07/04/11 23:01, Robert Haas wrote:

On Wed, Apr 6, 2011 at 3:37 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Add traceback information to PL/Python errors

This mimics the traceback information the Python interpreter prints
with exceptions.

Jan Urbański

On my system this spits out a warning:

plpython.c: In function ‘PLy_traceback’:
plpython.c:4487: warning: ‘s’ may be used uninitialized in this function
plpython.c:4487: note: ‘s’ was declared here

That appears to be a live bug, unless it's guaranteed that lineno will
always be > 0.

lineno should be > 0, unless Python is trying to tell us that the code
frame originates from before the function.

Also, the loop test should really be written as current < lineno,
rather than current != lineno, just in case we should manage to pass a
lineno < 0, which with the current code would go into the tank and
spin.

Yeah, good point.

This part looks pretty sketchy, too:

while (s && isspace((unsigned char) *s))
s++;

Perhaps we meant to test *s here. It's hard to believe that we're
really intending to test whether the pointer has fallen off the end of
the address space and wrapped around to NULL.

Gah, so short a function and so many things that I managed to get wrong.

Patch?

Attached.

There's also this:
http://archives.postgresql.org/pgsql-hackers/2011-04/msg00334.php

Yep. I am assuming Peter will look at that one.

I guess so. This only fixes the things you noticed.

Jan