Re: Failed assertion due to procedure created with SECURITY DEFINER option

> On Jul 4, 2018, at 3:43 AM, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>
> On 03.07.18 19:20, Andres Freund wrote:
>> On 2018-06-29 10:19:17 -0700, Andres Freund wrote:
>>> Hi,
>>>
>>> On 2018-06-29 13:56:12 +0200, Peter Eisentraut wrote:
>>>> On 6/29/18 13:07, amul sul wrote:
>>>>> This happens because of in fmgr_security_definer() function we are
>>>>> changing global variable SecurityRestrictionContext and in the
>>>>> StartTransaction() insisting it should be zero, which is the problem.
>>>>
>>>> Hmm, what is the reason for this insistation?
>>>
>>> Because it's supposed to be reset by AbortTransaction(), after an error.
>>
>> Does that make sense Peter?
>>
>> I've added this thread to the open items list.
>
> Proposed fix attached.

First, reproduced the issue against HEAD and was able to successfully
do so.

Then, applied the patch and tested using original test case:

testing=# CREATE PROCEDURE transaction_test1() LANGUAGE plpgsql SECURITY DEFINER
testing-# AS $$ BEGIN
testing$# COMMIT;
testing$# END $$;
CREATE PROCEDURE
testing=# CALL transaction_test1();
2018-07-12 15:45:49.846 EDT [39928] ERROR: invalid transaction termination
2018-07-12 15:45:49.846 EDT [39928] CONTEXT: PL/pgSQL function transaction_test1() line 2 at COMMIT
2018-07-12 15:45:49.846 EDT [39928] STATEMENT: CALL transaction_test1();
ERROR: invalid transaction termination
CONTEXT: PL/pgSQL function transaction_test1() line 2 at COMMIT

So it handles it as expected.

Code and test cases look fine to me from what I can tell. My only suggestion
would be if we could add some guidance to the documentation on what languages
can support transaction control statements inside procedures with a SECURITY
DEFINER.

Jonathan