MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Wed, 14 Feb 2018 19:27:11 +0300
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
Subject: Re: master plpython check fails on Solaris 10
Organization: Postgres Professional
In-Reply-To: <17568.1518620054@sss.pgh.pa.us>
References: <156b989dbc6fe7c4d3223cf51da61195@postgrespro.ru>
 <4a5588c7-9bcc-d5f5-b952-6b260467fc71@2ndquadrant.com>
 <2982e7d95f11eea5599f8141efbf4b2d@postgrespro.ru>
 <17568.1518620054@sss.pgh.pa.us>
Message-ID: <82168d2485db48a0267c1451ed357651@postgrespro.ru>
User-Agent: Roundcube Webmail/1.2.4
Precedence: bulk

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