Thread: PL/Perl embedding string common elements
The attached patch moves the common elements of loose_embedding[] and strict_embedding[] to a macro so they can be maintained in one place. As Tom Lane noticed, ::_plperl_to_pg_array was missing from strict_embedding[], which appears to be a bug. http://archives.postgresql.org/pgsql-bugs/2005-08/msg00189.php I'm not sure this patch meets Tom's request to avoid "too much violence to the readability," mostly because of the added backslashes at the end of the macro's lines. Opinions? -- Michael Fuhr
Attachment
Michael Fuhr <mike@fuhr.org> writes:
> The attached patch moves the common elements of loose_embedding[]
> and strict_embedding[] to a macro so they can be maintained in
> one place. As Tom Lane noticed, ::_plperl_to_pg_array was missing
> from strict_embedding[], which appears to be a bug.
Actually, it strikes me that this is working around a problem that
we shouldn't have in the first place: there should not be two different
embedding strings. I hadn't noticed the use_strict GUC before, but
now that I see it, I think it's implemented in a completely awful
way. Because the embedding string is only executed once per backend,
it's effectively impossible to change use_strict on the fly. I think
we should be looking for another implementation of that feature, rather
than just cleaning up the minor breakage. One would expect changes
in the variable to at least affect future function compilations in
the current backend, if not indeed recompiling already-compiled ones.
I'm not much of a Perl guru, but could we make this happen by having
four functions instead of two? (mksafefunc, mksafestrictfunc, etc)
I'm not too clear on what the scope of effects of 'use strict' is.
regards, tom lane
Tom Lane wrote: >Michael Fuhr <mike@fuhr.org> writes: > > >>The attached patch moves the common elements of loose_embedding[] >>and strict_embedding[] to a macro so they can be maintained in >>one place. As Tom Lane noticed, ::_plperl_to_pg_array was missing >>from strict_embedding[], which appears to be a bug. >> >> > >Actually, it strikes me that this is working around a problem that >we shouldn't have in the first place: there should not be two different >embedding strings. I hadn't noticed the use_strict GUC before, but >now that I see it, I think it's implemented in a completely awful >way. Because the embedding string is only executed once per backend, >it's effectively impossible to change use_strict on the fly. I think >we should be looking for another implementation of that feature, rather >than just cleaning up the minor breakage. One would expect changes >in the variable to at least affect future function compilations in >the current backend, if not indeed recompiling already-compiled ones. > >I'm not much of a Perl guru, but could we make this happen by having >four functions instead of two? (mksafefunc, mksafestrictfunc, etc) >I'm not too clear on what the scope of effects of 'use strict' is. > > The intention was to make it settable on startup only. Unfortunately when I tried that it blew up on me. It seems that the getting of custom settings happens too late for that to work. I recall posting a message to -hackers asking for some help on that, but got no response, so I just made it PGC_USERSET so I could make some progress. The scope of "use strict" is from the declaration down to the end of the enclosing block, file, or eval. The biggest problem is that "use" is in fact a forbidden operation in trusted plperl. If you wanted a simpler (and in a way more perlish) way of enabling this, we could abandon the GUC var altogether and load the strict module unconditionally. Then each user function could turn on strict mode at their choice by calling "strict->import();", possibly in a BEGIN block (plperlu functions could just have "use strict;"). Every perl module whose author wants strict mode (and they all should) has to carry such a declaration, so in a sense we'd just be doing what perl itself does, and by trying to provide a global switch we're being unperlish. Of course, many people regret that strict mode was not (mainly so that millions of lines of legacy code didn't break) made the default in perl5. :-) I know of one ardent plperl advocate at least who would be highly disappointed if we followed this course. We'd still have made an advance, because in 8.0 you can't turn on strict mode at all in trusted plperl. Of course, we're getting close to Beta, but if you're happy redesigning things at this stage, we should be able to wrap it up fairly quickly - it's hardly huge. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Every perl module whose author wants strict mode (and they all should)
> has to carry such a declaration, so in a sense we'd just be doing what
> perl itself does, and by trying to provide a global switch we're being
> unperlish.
You missed my point. I wasn't objecting to having the global switch,
only to the fact that turning it on and off doesn't do what a rational
person would expect. If it's going to advertise itself as USERSET then
it darn well ought to be settable.
The idea of loading the strict module unconditionally seems ok to me,
if we can work out a way of making it apply or not apply to individual
function compilations. From what you were saying, perhaps it would
work to implicitly add "strict->import();" when use_strict is enabled?
regards, tom lane
Tom Lane wrote:
>Andrew Dunstan <andrew@dunslane.net> writes:
>
>
>>Every perl module whose author wants strict mode (and they all should)
>>has to carry such a declaration, so in a sense we'd just be doing what
>>perl itself does, and by trying to provide a global switch we're being
>>unperlish.
>>
>>
>
>You missed my point. I wasn't objecting to having the global switch,
>only to the fact that turning it on and off doesn't do what a rational
>person would expect. If it's going to advertise itself as USERSET then
>it darn well ought to be settable.
>
>The idea of loading the strict module unconditionally seems ok to me,
>if we can work out a way of making it apply or not apply to individual
>function compilations. From what you were saying, perhaps it would
>work to implicitly add "strict->import();" when use_strict is enabled?
>
>
>
>
I am pretty sure we could load it unconditionally without disturbing
anything (because of the scope rules), and then import it appropriately.
Do you expect turning it on to affect only future compilations? Or
should we recompile every function already compiled in the present
backend? I can see arguments either way. What about if we turn it off?
In theory, anything that works with it turned on should work fine with
it turned off, although the reverse is not true of course ...
And yes, you're right, we could have 4 functions instead of two:
{safe,unsafe} x {strict, unstrict}.
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Do you expect turning it on to affect only future compilations? Or
> should we recompile every function already compiled in the present
> backend? I can see arguments either way.
Yeah, me too. I would definitely expect a change in the variable
(in either direction) to be respected in subsequent function
compilations. I'm less excited about redoing previous compilations;
a case could be made for doing so, but I won't insist on it.
I think the main case where use_strict is interesting is in development,
where you're repeatedly CREATE OR REPLACE'ing the function and retesting
it, so you're going to be doing new compilations anyway.
Looking ahead to the future a bit, is there any reason to expect that
use_strict would have cross-function effects? In a normal Perl script
I suppose it does add some cross-function checking. Right now, with
plperl functions all mutually anonymous, there are no interactions to
be strict about --- but it says in the todo list that that's something
to fix someday. When that happens, would it actually be correct or
even essential to force use_strict to have just one value throughout a
backend's lifespan? If so, the existing code isn't so unreasonable,
but we need to fix the docs ...
regards, tom lane
Tom Lane wrote: >Looking ahead to the future a bit, is there any reason to expect that >use_strict would have cross-function effects? In a normal Perl script >I suppose it does add some cross-function checking. > Not sure what you mean by this. Perl strict mode in fact does very little checking on function calls - the existence and visibility of the function is checked at call time, not compile time. This can be a bit annoying, especially if you're switching backwards and forwards between perl and some language that is less, er, forgiving, but it's essential to the way perl works. >Right now, with >plperl functions all mutually anonymous, there are no interactions to >be strict about --- but it says in the todo list that that's something >to fix someday. When that happens, would it actually be correct or >even essential to force use_strict to have just one value throughout a >backend's lifespan? If so, the existing code isn't so unreasonable, >but we need to fix the docs ... > > > > It might well say that in the TODO, but actually doing it is another matter entirely. I spent a long time looking at this. Quite apart from issues of name mangling to get plperl and SQL names aligned, there is the fact that, with the exception of CREATE OR REPLACE FUNCTION, we only compile a function when it is first called by SQL. So we'd need to ensure that every known function was compiled in every backend for which it is visible in case we need to call it directly. There might be some way that I can't see to do it - I'm meeting a few perl gurus in a few weeks and I'll ask if I get the chance. Also, we'd need to work out a way for custom GUCs to work on startup only - I know it broke for me. So let's just design for what we have now ;-) cheers andrew
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 > The biggest problem is that "use" is in fact a forbidden operation in > trusted plperl. I would very much like to have use strict available in plperl. Is there any way at all to simply make an exception for 'use strict' (and perhaps some other pragma) without going through more contortions? I can't imagine enabling it would in any way make it more "untrusted". On the other hand, if 'use strict' was enabled for plperl, I might never have discovered the bug at the start of this thread. :) - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 200508212024 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iD8DBQFDCRu9vJuQZxSWSsgRAhRQAKDRupUuZ6CPPVliNliHS31m3koS4QCfS9hs CKltu0fUbghaT/hZ1HsBhRQ= =5axN -----END PGP SIGNATURE-----
Greg Sabino Mullane said: > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > >> The biggest problem is that "use" is in fact a forbidden operation in >> trusted plperl. > > I would very much like to have use strict available in plperl. You're not alone. That's why we're going through all this. > Is there > any way at all to simply make an exception for 'use strict' (and > perhaps some other pragma) without going through more contortions? I > can't imagine enabling it would in any way make it more "untrusted". > "use" even for pragmas in unfortunately a "require" op, which makes it quite definitely unsafe. There are just some things that we have to forego in trusted plperl. We have worked around a couple of the biggest (strict, warnings, utf8) and can handle others on a case by case basis. If you have a better way I'm all ears. cheers andrew
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>Do you expect turning it on to affect only future compilations? Or >>should we recompile every function already compiled in the present >>backend? I can see arguments either way. >> >> > >Yeah, me too. I would definitely expect a change in the variable >(in either direction) to be respected in subsequent function >compilations. I'm less excited about redoing previous compilations; >a case could be made for doing so, but I won't insist on it. >I think the main case where use_strict is interesting is in development, >where you're repeatedly CREATE OR REPLACE'ing the function and retesting >it, so you're going to be doing new compilations anyway. > > > > Discussion seems to have died on this. If there's no objection and nobody else is doing this I will prepare a patch based on keeping the setting as USERSET and not recompiling previously compiled functions. I'll move the embedding strings to macroland and tidy things up as discussed. It will take me a few days to get that ready. cheers andrew