Skip site navigation (1) Skip section navigation (2)

Re: Plperl trigger variables no longer global

From: Alex Hunsaker <badalex(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Greg Sabino Mullane <greg(at)endpoint(dot)com>, pgsql-bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: Plperl trigger variables no longer global
Date: 2011-05-16 18:57:41
Message-ID: BANLkTi=PRK5xsoWcWkjuGoW8=Mpy4ThCcQ@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-bugs
On Sun, May 15, 2011 at 14:02, Robert Haas <robertmhaas(at)gmail(dot)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.

In response to

Responses

pgsql-bugs by date

Next:From: dhavalDate: 2011-05-17 14:30:37
Subject: BUG #6027: OLE DB Provider not available
Previous:From: Bruce MomjianDate: 2011-05-16 15:22:18
Subject: Re: BUG #6026: redundant sentences?

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group