> On Jul 4, 2018, at 3:43 AM, Peter Eisentraut > <peter.eisentr...@2ndquadrant.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
