Re: Plperl trigger variables no longer global

On Sun, May 15, 2011 at 14:02, Robert Haas <robertmhaas@gmail.com> wrote:

>> Fixed in the attached plus added regression tests for both issues (use
>> strict; && Global symbol "$_TD" requires explicit package name, test
>> recursive trigger calls). Although Ill admit, given the point we are
>> in the release I could see a revert also being justified.
>>
>> Greg, big thanks for testing! keep it up! :)
>
> Do we need to apply this patch?

Only if we want use strict and trigger functions to work in 9.1 :). I
realize I did a poor job of explaining the problem and the solution,
that probably made it hard for a -commiter to pick up. Here is a stab
at that.

[ pgcon is spinning up so I won't hold my breath waiting for a response ]

Problem:

CREATE FUNCTION wheredidmytdgo()
RETURNS TRIGGER
LANGUAGE plperlu
AS
$bc$
   use strict;
   my $new = $_TD->{new};
   return;
$bc$;

fails with ERROR:  Global symbol "$_TD" requires explicit package name
at line 3.

Analysis:

The above fails due to
http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=ef19dc6d39dd2490ff61489da55d95d6941140bf.
It moved declaring of $_TD from plperl_create_sub to
plperl_call_perl_trigger_func(). Before it was always declared even
for non trigger functions. This broke CREATE FUNCTION validation as
$_TD has not been declared. Assuming you could get past the validator
things would still not work, plperl_call_trigger_func() tried to
declare it using get_sv("_TD", GV_ADD) which seems like it should
work, but does not[1].

We (I) failed to notice this during review as it only shows when
running under "use strict", which apparently is uncommon.

Fix:

My proposed fix is instead of declaring $_TD in
plperl_call_perl_trigger_func(), just declare it as a global the same
way we declare %_SHARED. It keeps things simple and we should get to
keep any performance benefits from the patch.

While playing with it I also noticed it was failing to be local()ized
properly meaning nested trigger functions would clobber $_TD, also fix
that.

Also added regression tests for both issues.

--
(1) If it was working and creating a global correctly you should be
able to call a trigger function so that $_TD gets declared and then
create a new trigger function that uses strict and references $_TD
without any problems. But it doesn't work...

# create or replace function td() returns trigger language plperlu as
$bc$ return; $bc$;
CREATE FUNCTION
# create table trig_test(a int);
CREATE TABLE
# create trigger test_trig after insert on trig_test execute procedure td();
CREATE TRIGGER
# insert into trig_test values (1);
INSERT 0 1
# CREATE or replace FUNCTION wheredidmytdgo()
RETURNS TRIGGER
LANGUAGE plperlu
AS
$bc$
   use strict;
   my $new = $_TD->{new};
   return;
$bc$;
ERROR:  Global symbol "$_TD" requires explicit package name at line 3.