Re: master plpython check fails on Solaris 10

Marina Polyakova <m.polyakova@postgrespro.ru>

From: Marina Polyakova <m.polyakova@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, pgsql-hackers@postgresql.org
Date: 2018-02-14T16:27:11Z
Lists: pgsql-hackers
On 14-02-2018 17:54, Tom Lane wrote:
> Marina Polyakova <m.polyakova@postgrespro.ru> writes:
>> On 14-02-2018 3:43, Peter Eisentraut wrote:
>>> OK, can you get some kind of stack trace or other debugging
>>> information?
> 
>> I got this backtrace from gdb:
> 
> Hmm, so the only question in my mind is how did this ever work for 
> anyone?
> 
> The basic problem here is that, instead of using the
> ErrorContextCallback.arg field provided for the purpose,
> plpython_error_callback is using PLy_current_execution_context()
> to try to identify the context it's supposed to report on.
> In the example, that points to the context associated with the
> inner DO block, not with the outer procedure.  That context
> looks like it should reliably have curr_proc->proname == NULL,
> so how come this test case doesn't crash for everybody?
> 
> In any case the expected output for the transaction_test4
> case is obviously wrong.  Rather than reporting the transaction_test4
> function and then the inner DO block, it's reporting 2 levels
> of transaction_test4.  That seems to trace directly to both levels
> of error context callback looking at the same execution context.
> 
> I think we need to fix the error callback code so that it
> uses the "arg" field to find the relevant procedure, and that
> that's a back-patchable fix, because nested plpython functions
> would show this up as wrong in any case.  That would also let us
> undo the not-terribly-safe practice of having code between the
> PLy_push_execution_context call and the PG_TRY that is supposed
> to ensure the context gets popped again.

Thank you very much! I'll try to implement this.

> While I'm bitching ... I wonder how long it's been since the
> comment for PLy_procedure_name() had anything to do with its
> actual behavior.

AFAIU this has always been like that, since the commit 1ca717f3...

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


Commits

  1. Add an assertion that we don't pass NULL to snprintf("%s").

  2. Fix broken logic for reporting PL/Python function names in errcontext.

  3. Fix typo

  4. Transaction control in PL procedures

  5. Fix docs typo

  6. plpython security and error handling fixes, from