On 18/01/11 23:22, Peter Eisentraut wrote: > On mån, 2011-01-17 at 21:49 +0200, Peter Eisentraut wrote: >> On sön, 2011-01-02 at 12:41 +0100, Jan Urbański wrote: >>> Here they are. There are 16 patches in total. They amount to what's >>> currently in my refactor branch (and almost to what I've sent as the >>> complete refactor patch, while doing the splitting I discovered a few >>> useless hunks that I've discarded). >> >> Committed 0001, rest later. > > Today committed: 3, 5, 10, 11, 12, 13
Thanks, > #2: It looks like this loses some information/formatting in the error > message. Should we keep the pointing arrow there? > > --- a/src/pl/plpython/expected/plpython_error.out > +++ b/src/pl/plpython/expected/plpython_error.out > @@ -10,10 +10,7 @@ CREATE FUNCTION sql_syntax_error() RETURNS text > SELECT sql_syntax_error(); > WARNING: PL/Python: plpy.SPIError: unrecognized error in > PLy_spi_execute_query > CONTEXT: PL/Python function "sql_syntax_error" > -ERROR: syntax error at or near "syntax" > -LINE 1: syntax error > - ^ > -QUERY: syntax error > +ERROR: PL/Python: plpy.SPIError: syntax error at or near "syntax" > CONTEXT: PL/Python function "sql_syntax_error" > /* check the handling of uncaught python exceptions > */ Yes, the message is less informative, because the error is reported by PL/Python, by wrapping the SPI message. I guess I could try to extract more info from the caught ErrorData and put it in the new error that PL/Python throws. But I'd like to do that as a refinement of the spi-in-subxacts patch, because the current behaviour is broken anyway - to catch the SPI error safely you need to wrap the whole thing in a subtransaction. This patch only gets rid of the global error state, and to do that I needed to raise some exception from the try/catch block around SPI calls. > #7: This is unnecessary because the remaining fields are automatically > initialized with NULL. The Python documentation uses initializations > like that. The way I understand it, newer Python versions might add > more fields at the end, and they will rely on the fact that they get > automatically initialized even if the source code was for an older > version. So I would rather not touch this, as it doesn't buy anything. OK. > #16: This is probably pointless because pgindent will reformat this. If it does, than it's a shame. Maybe the comment should be moved above the if() then. 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