Re: [PATCHES] plperl - make $_TD global
Andrew Dunstan wrote: > >>The attached tiny patch will fix the problem Greg Sabino Mullane had > >>with a shared lexical $_TD, by making it a global and just pushing a > >>local value in the trigger function. > >>... > >>I don't think a docs change is needed. > > > >Are you sure this doesn't cause any user-visible semantics change > >(ie, something that might break someone's code)? > > > > > > > > > > I think it should be mentioned in the release notes. > > I would be fairly stretched to come up with an example that it breaks. > Essentially it's a way around the sharing violation that Greg got from > using $_TD in a nested subroutine. All we are doing is moving $_TD from > the local namespace to the global namespace. It still gets a per > trigger-call value (that's what the "local $_TD = $_[0];" does) and that > will work correctly even under recursive calls, so I think it's fairly safe. > > Maybe it is worth putting somethng in the plperl Trigger docs about the > nature of the beast. I will do that if you like. > Yes, I think something for docs would be good. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] plperl - make $_TD global
Tom Lane wrote: Andrew Dunstan <[EMAIL PROTECTED]> writes: The attached tiny patch will fix the problem Greg Sabino Mullane had with a shared lexical $_TD, by making it a global and just pushing a local value in the trigger function. ... I don't think a docs change is needed. Are you sure this doesn't cause any user-visible semantics change (ie, something that might break someone's code)? I think it should be mentioned in the release notes. I would be fairly stretched to come up with an example that it breaks. Essentially it's a way around the sharing violation that Greg got from using $_TD in a nested subroutine. All we are doing is moving $_TD from the local namespace to the global namespace. It still gets a per trigger-call value (that's what the "local $_TD = $_[0];" does) and that will work correctly even under recursive calls, so I think it's fairly safe. Maybe it is worth putting somethng in the plperl Trigger docs about the nature of the beast. I will do that if you like. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] plperl - make $_TD global
Andrew Dunstan <[EMAIL PROTECTED]> writes: > The attached tiny patch will fix the problem Greg Sabino Mullane had > with a shared lexical $_TD, by making it a global and just pushing a > local value in the trigger function. > ... > I don't think a docs change is needed. Are you sure this doesn't cause any user-visible semantics change (ie, something that might break someone's code)? regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] plperl - make $_TD global
The attached tiny patch will fix the problem Greg Sabino Mullane had
with a shared lexical $_TD, by making it a global and just pushing a
local value in the trigger function.
I don't think what we had is strictly a bug, so I don't thinbk we need
top backpatch this.
It will, however, require use of perl 5.6 at a minimum, because that's
when the "our" function came in. Since that was over 6 years ago, I
think this is not unreasonable. If there are squawks, I have another
slightly longer and slightly more old-fashioned way to do the same
thing, but this is the best modern way.
I don't think a docs change is needed.
If there's no objection I will apply thin in a few days.
cheers
andrew
Index: src/pl/plperl/plperl.c
===
RCS file: /cvsroot/pgsql/src/pl/plperl/plperl.c,v
retrieving revision 1.108
diff -c -r1.108 plperl.c
*** src/pl/plperl/plperl.c 4 Apr 2006 19:35:37 - 1.108
--- src/pl/plperl/plperl.c 22 May 2006 20:46:37 -
***
*** 765,771
ENTER;
SAVETMPS;
PUSHMARK(SP);
! XPUSHs(sv_2mortal(newSVpv("my $_TD=$_[0]; shift;", 0)));
XPUSHs(sv_2mortal(newSVpv(s, 0)));
PUTBACK;
--- 765,771
ENTER;
SAVETMPS;
PUSHMARK(SP);
! XPUSHs(sv_2mortal(newSVpv("our $_TD; local $_TD=$_[0]; shift;", 0)));
XPUSHs(sv_2mortal(newSVpv(s, 0)));
PUTBACK;
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
