Thread: 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 -0000 1.108 --- src/pl/plperl/plperl.c 22 May 2006 20:46:37 -0000 *************** *** 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;
Andrew Dunstan <andrew@dunslane.net> 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
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> 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
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 EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +