On Thu, May 5, 2011 at 06:51, Alvaro Herrera <[email protected]> wrote:
> Excerpts from Alex Hunsaker's message of mié may 04 23:53:34 -0300 2011:
>
>> After playing with it a bit more I see 2 clear options:
>> 1) make $_TD global like %_SHARED. This should not cause any problems
>> as we make $_TD private via local() before each trigger call. Also pre
>> 9.1 non trigger functions could still access and check the definedness
>> of $_TD so if someone was relying on that (for whatever unknown
>> reason) that will work again.
>
> This is strange. Are you saying that there's no decent way to make a
> variable global in C code?
Im sure we could... I don't see any reason to do it in C. (performance
or otherwise)
In other news I found another bug with this-- it was trying to
local($_TD) by using SAVESPTR() when it seems it really should be
using save_item(). Currently its not really localizing $_TD, which at
the very least means recursive triggers might modify the callers $_TD.
Ugh.
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! :)
*** a/src/pl/plperl/expected/plperl_trigger.out
--- b/src/pl/plperl/expected/plperl_trigger.out
***************
*** 255,260 **** SELECT * FROM trigger_test;
--- 255,289 ----
5 | third line(modified by trigger)(modified by trigger) | ("(5)")
(4 rows)
+ DROP TRIGGER "test_valid_id_trig" ON trigger_test;
+ CREATE OR REPLACE FUNCTION trigger_recurse() RETURNS trigger AS $$
+ use strict;
+
+ if ($_TD->{new}{i} == 10000)
+ {
+ spi_exec_query("insert into trigger_test (i, v) values (20000, 'child');");
+
+ if ($_TD->{new}{i} != 10000)
+ {
+ die "recursive trigger modified: ". $_TD->{new}{i};
+ }
+ }
+ return;
+ $$ LANGUAGE plperl;
+ CREATE TRIGGER "test_trigger_recurse" BEFORE INSERT ON trigger_test
+ FOR EACH ROW EXECUTE PROCEDURE "trigger_recurse"();
+ INSERT INTO trigger_test (i, v) values (10000, 'top');
+ SELECT * FROM trigger_test;
+ i | v | foo
+ -------+------------------------------------------------------+---------
+ 1 | first line(modified by trigger) | ("(2)")
+ 2 | second line(modified by trigger) | ("(3)")
+ 4 | immortal | ("(4)")
+ 5 | third line(modified by trigger)(modified by trigger) | ("(5)")
+ 20000 | child |
+ 10000 | top |
+ (6 rows)
+
CREATE OR REPLACE FUNCTION immortal() RETURNS trigger AS $$
if ($_TD->{old}{v} eq $_TD->{args}[0])
{
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***************
*** 1,7 ****
# src/pl/plperl/plc_perlboot.pl
use 5.008001;
! use vars qw(%_SHARED);
PostgreSQL::InServer::Util::bootstrap();
--- 1,7 ----
# src/pl/plperl/plc_perlboot.pl
use 5.008001;
! use vars qw(%_SHARED $_TD);
PostgreSQL::InServer::Util::bootstrap();
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***************
*** 1976,1983 **** plperl_call_perl_trigger_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo,
ENTER;
SAVETMPS;
! TDsv = get_sv("_TD", GV_ADD);
! SAVESPTR(TDsv); /* local $_TD */
sv_setsv(TDsv, td);
PUSHMARK(sp);
--- 1976,1986 ----
ENTER;
SAVETMPS;
! TDsv = get_sv("_TD", 0);
! if(!TDsv)
! elog(ERROR, "couldn't fetch $_TD");
!
! save_item(TDsv); /* local $_TD */
sv_setsv(TDsv, td);
PUSHMARK(sp);
*** a/src/pl/plperl/sql/plperl_trigger.sql
--- b/src/pl/plperl/sql/plperl_trigger.sql
***************
*** 122,127 **** UPDATE trigger_test SET i = 100 where i=1;
--- 122,151 ----
SELECT * FROM trigger_test;
+ DROP TRIGGER "test_valid_id_trig" ON trigger_test;
+
+ CREATE OR REPLACE FUNCTION trigger_recurse() RETURNS trigger AS $$
+ use strict;
+
+ if ($_TD->{new}{i} == 10000)
+ {
+ spi_exec_query("insert into trigger_test (i, v) values (20000, 'child');");
+
+ if ($_TD->{new}{i} != 10000)
+ {
+ die "recursive trigger modified: ". $_TD->{new}{i};
+ }
+ }
+ return;
+ $$ LANGUAGE plperl;
+
+ CREATE TRIGGER "test_trigger_recurse" BEFORE INSERT ON trigger_test
+ FOR EACH ROW EXECUTE PROCEDURE "trigger_recurse"();
+
+ INSERT INTO trigger_test (i, v) values (10000, 'top');
+
+ SELECT * FROM trigger_test;
+
CREATE OR REPLACE FUNCTION immortal() RETURNS trigger AS $$
if ($_TD->{old}{v} eq $_TD->{args}[0])
{
--
Sent via pgsql-bugs mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
