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 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
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.
From | Date | Subject | |
---|---|---|---|
Next Message | dhaval | 2011-05-17 14:30:37 | BUG #6027: OLE DB Provider not available |
Previous Message | Bruce Momjian | 2011-05-16 15:22:18 | Re: BUG #6026: redundant sentences? |