Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https
On Mon, Aug 08, 2011 at 01:23:08AM -0600, Alex Hunsaker wrote:
On Sun, Aug 7, 2011 at 17:06, Tim Bunce tim.bu...@pobox.com wrote:
Localizing an individual element of %SIG works fine.
In C that's something like this (untested):
hv = gv_fetchpv(SIG, 0, SVt_PVHV);
keysv = ...SV containing ALRM...
he = hv_fetch_ent(hv, keysv, 0, 0);
if (he) { /* arrange to restore existing elem */
save_helem_flags(hv, keysv, HeVAL(he), SAVEf_SETMAGIC);
}
else { /* arrange to delete a new elem */
SAVEHDELETE(hv, keysv);
}
I played with this a bit... and found yes, it locals them but no it
does not fix the reported problem. After playing with things a bit
more I found even local $SIG{'ALRM'} = .,..; alarm(1); still results
in postgres crashing. To wit, local does squat. AFAICT it just resets
the signal handler back to the default with SIG_DFL. (Which in
hindsight I don't know what else I expected it to-do...)
Ah, yes. Hindsight is great. I should have spotted that. Sorry.
So I think for this to be robust we would have to detect what signals
they set and then reset those back to what postgres wants. Doable, but
is it worth it? Anyone else have any bright ideas?
I'm only considering ALRM. At least that's the only one that seems worth
offering some limited support for. The others fall under don't do that.
After giving it some more thought it seems reasonable to simply force the
SIGALRM handler back to postgres when a plperlu function returns:
pqsignal(SIGALRM, handle_sig_alarm);
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https
[I've included a summary of the thread and Bcc'd this to perl5-porters
for a sanity check. Please trim heavily when replying.]
On Thu, Aug 04, 2011 at 09:42:31AM -0400, Andrew Dunstan wrote:
So doesn't this look like a bug in the perl module that sets the
signal handler and doesn't restore it?
What happens if you wrap the calls to the module like this?:
{
local $SIG{ALRM};
# do LWP stuff here
}
return 'OK';
That should restore the old handler on exit from the block.
I think if you use a perl module that monkeys with the signal
handlers for any signal postgres uses all bets are off.
On Thu, Aug 04, 2011 at 10:28:45AM -0400, Tom Lane wrote:
Sure, but how expensive would it be for pl/perl to do this
automatically ?
How can anything like that possibly work with any reliability
whatsoever? If the signal comes in, you don't know whether it was
triggered by the event Postgres expected, or the event the perl module
expected, and hence there's no way to deliver it to the right signal
handler (not that the code you're describing is even trying to do that).
What *I'd* like is a way to prevent libperl from touching the host
application's signal handlers at all. Sadly, Perl does not actually
think of itself as an embedded library, and therefore thinks it owns all
resources of the process and can diddle them without anybody's
permission.
The PERL_IMPLICIT_SYS mechanism addresses this. Unfortunately it only
works with USE_ITHREADS on Windows currently.
http://perldoc.perl.org/perlguts.html#Future-Plans-and-PERL_IMPLICIT_SYS
On Thu, Aug 04, 2011 at 04:09:47PM -0600, Alex Hunsaker wrote:
Well we can't prevent perl XS (aka C) from messing with signals (and
other modules like POSIX that expose things like sigprocmask,
siglongjump etc.) , but we could prevent plperl(u) from playing with
signals on the perl level ala %SIG.
[ IIRC I proposed doing something about this when we were talking
about the whole Safe mess, but I think there was too much other
discussion going on at the time :-) ]
Mainly the options im thinking about are:
1) if anyone touches %SIG die
2) turn %SIG into a regular hash so people can set/play with %SIG, but
it has no real effect.
3) local %SIG before we call their trigger function. This lets signals
still work while in trigger scope (like we do for %_TD)
4) if we can't get any of the above to work we can save each %SIG
handler before and restore them after each trigger call. (mod_perl
does something similar so Im fairly certain we should be able to get
that to work)
On Thu, Aug 4, 2011 at 16:34, David E. Wheeler da...@kineticode.com wrote:
1) if anyone touches %SIG die
2) turn %SIG into a regular hash so people can set/play with %SIG, but
it has no real effect.
These would disable stuff like $SIG{__WARN__} and $SIG{__DIE__}, which would
be an unfortunate side-effect.
On Thu, Aug 04, 2011 at 07:52:45PM -0400, Tom Lane wrote:
The scenario I was imagining was:
1. perl temporarily takes over SIGALRM.
2. while perl function is running, statement_timeout expires, causing
SIGALRM to be delivered.
3. perl code is probably totally confused, and even if it isn't,
statement_timeout will not be enforced since Postgres won't ever get the
interrupt.
Even if you don't think statement_timeout is a particularly critical
piece of functionality, similar interference with the delivery of, say,
SIGUSR1 would be catastrophic.
How do you propose to prevent this sort of problem?
I don't think there's complete solution for that particular scenario.
[Though redirecting the perl alarm() opcode to code that would check the
argument against the remaining seconds before statement_timeout expires,
might get close.]
On Thu, Aug 04, 2011 at 06:44:18PM -0600, Alex Hunsaker wrote:
How do you propose to prevent this sort of problem?
Well, I think that makes it unworkable.
So back to #1 or #2.
For plperlu sounds like we are going to need to disallow setting _any_
signals (minus __DIE__ and __WARN__). I should be able to make it so
when you try it gives you a warning something along the lines of
plperl can't set signal handlers, ignoring
For plperl I think we should probably do the same. It seems like
Andrew might disagree though? Anyone else want to chime in on if
plperl lets you muck with signal handlers?
Im not entirely sure how much of this is workable, I still need to go
through perl's guts and see. At the very worst I think we can mark
each signal handler that is exposed in %SIG readonly (which would mean
we would die instead of warning), but I think I can make the warning
variant workable as well.
I also have not dug deep enough to know how to handle __WARN__ and
__DIE__ (and exactly what limitations allowing those will impose). I
still have some work at $day_job before I can really dig into this.
On Thu, Aug 04, 2011 at 09:40:57PM -0400, Andrew Dunstan wrote:
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Sat, Feb 12, 2011 at 02:17:12AM +0200, Alexey Klyukin wrote: So, here is the v8. Instead of rewriting the encode_array_literal I've added another function, encode_type_literal (could use a better name). Given that encode_array_literal() encodes an _array_ as a literal, I'd assume encode_type_literal() encodes a _type_ as a literal. I'd suggest encode_typed_literal() as a better name. Tim [just passing though] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 08, 2011 at 09:40:38AM -0500, Andrew Dunstan wrote: On 02/03/2011 01:20 PM, Andrew Dunstan wrote: Well, the question seems to be whether or not it's a reasonable price to pay. On the whole I'm inclined to think it is, especially when it can be avoided by updating your code, which will be a saving in fragility and complexity as well. do you till have concerns about this, or are you happy for us to move ahead on it? [I'm not really paying close enough attention for you to put much weight on my opinions, but...] I can't see any major issues so I'm happy for you to move ahead. Thanks! Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Wed, Feb 02, 2011 at 12:10:59PM -0500, Andrew Dunstan wrote: On 02/02/2011 11:45 AM, Tim Bunce wrote: But why are we bothering to keep $prolog at all any more, if all we're going to pass it isPL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. PostgreSQL::PLPerl::NYTProf would break if it was removed, so I'd rather it wasn't. I could work around that if there's an easy way for perl code to tell what version of PostgreSQL. If there isn't I think it would be worth adding. Not really. It might well be good to add but that needs to wait for another time. Ok. I gather you're plugging in a replacement for mkfunc? Yes. For now I'll add a comment to the code saying why it's there. Thanks. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 03, 2011 at 02:23:32PM +0200, Alexey Klyukin wrote:
Hi,
On Feb 2, 2011, at 7:16 PM, Tim Bunce wrote:
I'm sorry I'm late to this party. I haven't been keeping up with
pgsql-hackers.
Better late than never :)
I'd kind'a hoped that this functionality could be tied into extending
PL/Perl to handle named arguments. That way the perl variables
corresponding to the named arguments could be given references without
breaking any code.
Franky I don't see a direct connection between conversion of arrays
into array references and supporting named arguments.
There is no direct connection. I wasn't clear, tied was too strong a
word for it.
Could you, please, show some examples of how you hoped the
functionality could be extended?
I wasn't suggesting extending the functionality, just a way of adding
the functionality that wouldn't risk impacting existing code.
Imagine that PL/Perl could handle named arguments:
CREATE FUNCTION join_list( separator text, list array ) AS $$
return join( $separator, @$list );
$$ LANGUAGE plperl;
The $list variable, magically created by PL/Perl, could be the array
reference created by your code, without altering the contents of @_.
Existing code that does the traditional explicit unpacking of @_
wouldn't be affected. So there would be no need for the string overload
hack which, although I suggested it, I'm a little uncomfortable with.
(The problems with recursion and the need for is_array_ref with
hardwired class names are a little troubling. Not to mention the
extra overheads in accessing the array.)
On the ther hand, a string version of the array would still need to be
created for @_.
Some observations on the current code (based on a quick skim):
- I'd like to see the conversion function exposed as a builtin
$ref = decode_array_literal({...});
In principal, I think that's not hard to built with the functionality provided
by this patch. I see this as an XS function which takes the array string,
calls the array_in to convert it to the array datum, and, finally, calls
plperl_ref_from_pg_array (provided by the patch) to produce the resulting
array reference.
I think that would be useful.
- Every existing plperl function that takes arrays is going to get
slower due to the overhead of parsing the string and allocating the
array and all its elements.
Well, per my understanding of Alex changes, the string parsing is not invoked
unless requested by referencing the array in a string context. Normally, onle
plperl_ref_from_pg_array will be invoked every time the function is called
with an array argument, which would take little time to convert the PostgreSQL
internal array representation (not a string) to the perl references, but
that's
no different from what is already done with composite type arguments, which
are converted to perl hash references on every corresponding function call.
I'd missed that it was using the internal array representation (obvious
in hindsight) but there's still a significant cost in allocating the SVs
that won't be used by existing code. Though I agree it's of the same
order as for composite types.
- Some of those functions may not use the array at all and some may
simply pass it on as an argument to another function.
I don't see how it would be good to optimize for the functions that are
declared to get the array but in fact do nothing with it. And considering the
case of passing an array through to another function, it's currently not
possible to call another pl/perl function from an existing one directly, and
when passing muti-dimensional arrays to a perl function one would need to
convert it to the array references anyway.
I was thinking of calls to other pl/perl functions via sql.
- Making the conversion lazy would be a big help.
Converting it to string is already lazy, and, per the argumens above, I don't
see a real benefit of lazy conversion to the perl reference (as comparing to
the hurdles of implementing that).
I (now) agree. Thanks.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Mon, Jan 31, 2011 at 02:22:37PM -0500, Andrew Dunstan wrote: On 01/15/2011 12:31 AM, Alex Hunsaker wrote: On Tue, Dec 7, 2010 at 07:24, Tim Buncetim.bu...@pobox.com wrote: Changes: Sets the local $_TD via C instead of passing an extra argument. So functions no longer start with our $_TD; local $_TD = shift; Pre-extend stack for trigger arguments for slight performance gain. Passes installcheck. Cool, surprisingly in the non trigger case I saw up to an 18% speedup. That's great. The trigger case remained about the same, I suppose im I/O bound. Find attached a v2 with some minor fixes, If it looks good to you Ill mark this as Ready for Commit. Changes: - move up a declaration to make it c90 safe - avoid using tg_trigger before it was initialized - only extend the stack to the size we need (there was + 1 which unless I am missing something was needed because we used to push $_TD on the stack, but we dont any more) This looks pretty good. But why are we bothering to keep $prolog at all any more, if all we're going to pass it is PL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. PostgreSQL::PLPerl::NYTProf would break if it was removed, so I'd rather it wasn't. I could work around that if there's an easy way for perl code to tell what version of PostgreSQL. If there isn't I think it would be worth adding. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
I'm sorry I'm late to this party. I haven't been keeping up with pgsql-hackers.
I'd kind'a hoped that this functionality could be tied into extending
PL/Perl to handle named arguments. That way the perl variables
corresponding to the named arguments could be given references without
breaking any code.
Some observations on the current code (based on a quick skim):
- I'd like to see the conversion function exposed as a builtin
$ref = decode_array_literal({...});
- Every existing plperl function that takes arrays is going to get
slower due to the overhead of parsing the string and allocating the
array and all its elements.
- Some of those functions may not use the array at all and some may
simply pass it on as an argument to another function.
- Making the conversion lazy would be a big help.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Wed, Dec 08, 2010 at 09:21:05AM -0800, David E. Wheeler wrote:
On Dec 8, 2010, at 9:14 AM, Tim Bunce wrote:
Do you have any more improvements in the pipeline?
I'd like to add $arrayref = decode_array_literal('{2,3}') and
maybe $hashref = decode_hstore_literal('x=1, y=2').
I don't know how much works would be involved in those though.
Those would be handy, but for arrays, at least, I'd rather have a GUC
to turn on so that arrays are passed to PL/perl functions as array
references.
Understood. At this stage I don't know what the issues are so I'm
nervous of over promising (plus I don't know how much time I'll have).
It's possible a blessed ref with string overloading would avoid
backwards compatibility issues.
Tim.
Another possible improvement would be rewriting encode_array_literal()
in C, so returning arrays would be much faster.
+1
Best,
David
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Tue, Dec 07, 2010 at 10:00:28AM -0500, Andrew Dunstan wrote:
On 12/07/2010 09:24 AM, Tim Bunce wrote:
Changes:
Sets the local $_TD via C instead of passing an extra argument.
So functions no longer start with our $_TD; local $_TD = shift;
Pre-extend stack for trigger arguments for slight performance gain.
Passes installcheck.
Please add it to the January commitfest.
Done. https://commitfest.postgresql.org/action/patch_view?id=446
Have you measured the speedup gained from this?
No. I doubt it's significant, but every little helps :)
Do you have any more improvements in the pipeline?
I'd like to add $arrayref = decode_array_literal('{2,3}') and
maybe $hashref = decode_hstore_literal('x=1, y=2').
I don't know how much works would be involved in those though.
Another possible improvement would be rewriting encode_array_literal()
in C, so returning arrays would be much faster.
I'd also like to #define PERL_NO_GET_CONTEXT, which would give a useful
performance boost by avoiding all the many hidden calls to lookup
thread-local storage. (PERL_SET_CONTEXT() would go and instead the
'currrent interpreter' would be passed around as an extra argument.)
That's a fairly mechanical change but the diff may be quite large.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Optimize PL/Perl function argument passing [PATCH]
Changes:
Sets the local $_TD via C instead of passing an extra argument.
So functions no longer start with our $_TD; local $_TD = shift;
Pre-extend stack for trigger arguments for slight performance gain.
Passes installcheck.
Tim.
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 5595baa..a924cfd 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*** plperl_create_sub(plperl_proc_desc *prod
*** 1421,1427
EXTEND(SP, 4);
PUSHs(sv_2mortal(newSVstring(subname)));
PUSHs(sv_2mortal(newRV_noinc((SV *) pragma_hv)));
! PUSHs(sv_2mortal(newSVstring(our $_TD; local $_TD=shift;)));
PUSHs(sv_2mortal(newSVstring(s)));
PUTBACK;
--- 1421,1427
EXTEND(SP, 4);
PUSHs(sv_2mortal(newSVstring(subname)));
PUSHs(sv_2mortal(newRV_noinc((SV *) pragma_hv)));
! PUSHs(PL_sv_no); /* unused */
PUSHs(sv_2mortal(newSVstring(s)));
PUTBACK;
*** plperl_call_perl_func(plperl_proc_desc *
*** 1495,1502
PUSHMARK(SP);
EXTEND(sp, 1 + desc-nargs);
- PUSHs(PL_sv_undef); /* no trigger data */
-
for (i = 0; i desc-nargs; i++)
{
if (fcinfo-argnull[i])
--- 1495,1500
*** plperl_call_perl_trigger_func(plperl_pro
*** 1583,1595
ENTER;
SAVETMPS;
! PUSHMARK(sp);
! XPUSHs(td);
tg_trigger = ((TriggerData *) fcinfo-context)-tg_trigger;
for (i = 0; i tg_trigger-tgnargs; i++)
! XPUSHs(sv_2mortal(newSVstring(tg_trigger-tgargs[i])));
PUTBACK;
/* Do NOT use G_KEEPERR here */
--- 1581,1596
ENTER;
SAVETMPS;
! SV *TDsv = get_sv(_TD, GV_ADD);
! SAVESPTR(TDsv); /* local $_TD */
! sv_setsv(TDsv, td);
! PUSHMARK(sp);
! EXTEND(sp, 1 + tg_trigger-tgnargs);
tg_trigger = ((TriggerData *) fcinfo-context)-tg_trigger;
for (i = 0; i tg_trigger-tgnargs; i++)
! PUSHs(sv_2mortal(newSVstring(tg_trigger-tgargs[i])));
PUTBACK;
/* Do NOT use G_KEEPERR here */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
PL/Perl function naming (was: [HACKERS] release notes)
[Sorry for the delay. I'd stopped checking the pgsql mailing lists.
Thanks to David Wheeler and Josh Berkus for the heads-up.]
On Sun, May 30, 2010 at 06:58:32PM -0400, Andrew Dunstan wrote:
Tim Bunce wrote:
On Mon, May 17, 2010 at 11:34:37AM -0400, Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Why do the release notes say this, under plperl:
* PL/Perl subroutines are now given names (Tim Bunce)
This is for the use of profiling and code coverage tools. DIDN'T
THEY HAVE NAMES BEFORE?
If whoever put this in the release notes had bothered
to ask I am sure we would have been happy to explain.
You don't need to complain, just fix it. The point of the comment is
that more explanation is needed.
I think you can just delete it. It's too esoteric to be worth noting.
Tim.
p.s. It also turned to be insufficiently useful for NYTProf since it
doesn't also update some internals to record the 'filename' and line
number range of the sub. So PostgreSQL::PLPerl::NYTProf works around
that by wrapping mkfuncsrc() to edit the generated perl code to include
a subname in the usual sub $name { ... } style. I may offer a patch
for 9.1 to to make it work that way.
I put this aside to think about it.
If the feature is not any use should we rip it out? We pretty much
included it because you said it was what you needed for the
profiler.
Yes, it can go.
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*** plperl_create_sub(plperl_proc_desc *prod
*** 1319,1328
(errmsg(didn't get a CODE ref from compiling
%s,
prodesc-proname)));
- /* give the subroutine a proper name in the main:: symbol table */
- CvGV(SvRV(subref)) = (GV *) newSV(0);
- gv_init(CvGV(SvRV(subref)), PL_defstash, subname, strlen(subname),
TRUE);
-
prodesc-reference = subref;
return;
I'm seriously nervous about adding function names - making functions
directly callable like that could be a major footgun recipe, since
now we are free to hide some stuff in the calling mechanism, and can
(and have in the past) changed that to suit our needs, e.g. when we
added trigger support via a hidden function argument. That has been
safe precisely because nobody has had a handle on the subroutine
which would enable it to be called directly from perl code. There
have been suggestions in the past of using this for other features,
so we should not assume that the calling mechanism is fixed in stone.
Agreed. It is a very hard to use footgun though because the oid is
included in the name. It's certainly not something anyone would shoot
themselves with by accident.
[Speaking of calling conventions: I never did get time for some decent
performance optimization. I'd like to get rid of the explicit extra
trigger data argument and corresponding local $_TD=shift; prologue.
That could be done much faster in C.]
Perhaps we could decorate the subs with attributes, or is that not
doable for anonymous subs?
Perhaps. I'll look into it when I get around to working on the PL/Perl
NYTProf again. For the profiler it would be enough to only enable the
naming when the appropriate perl debugging flag bits are set, so it
wouldn't happen normally.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] release notes
On Mon, May 17, 2010 at 11:34:37AM -0400, Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Why do the release notes say this, under plperl:
* PL/Perl subroutines are now given names (Tim Bunce)
This is for the use of profiling and code coverage tools. DIDN'T
THEY HAVE NAMES BEFORE?
If whoever put this in the release notes had bothered to ask I am sure
we would have been happy to explain.
You don't need to complain, just fix it. The point of the comment is
that more explanation is needed.
I think you can just delete it. It's too esoteric to be worth noting.
Tim.
p.s. It also turned to be insufficiently useful for NYTProf since it
doesn't also update some internals to record the 'filename' and line
number range of the sub. So PostgreSQL::PLPerl::NYTProf works around
that by wrapping mkfuncsrc() to edit the generated perl code to include
a subname in the usual sub $name { ... } style. I may offer a patch
for 9.1 to to make it work that way.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Core dump running PL/Perl installcheck with bleadperl [PATCH]
On Sun, Mar 07, 2010 at 12:11:26PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: I encountered a core dump running PL/Perl installcheck with a very recent git HEAD of PostgreSQL and a not quite so recent git HEAD of perl. The cause is a subtle difference between SvTYPE(sv) == SVt_RV and SvROK(sv). The former is checking a low-level implementation detail while the later is directly checking does this sv contains a reference. Hmm. Seems like this patch begs the question: if checking SvTYPE(*svp) isn't safe, why is it safe to look at SvTYPE(SvRV(*svp))? Shouldn't the tests against SVt_PVHV be made more abstract as well? Some SvTYPE values, like SVt_RV, allow the SV to hold one of a number of different kinds of things. Others, like SVt_PVHV, don't. No, I don't like it either but that's the way the Jenga tower made of yaks (to use a phrase recently coined by one of the perl maintainers) has grown. Something like an SvRVHVOK(sv) would be welcome sugar. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Safe security
On Wed, Mar 03, 2010 at 07:01:56PM -0500, Andrew Dunstan wrote:
Joshua D. Drake wrote:
On Wed, 2010-03-03 at 11:33 -0500, Andrew Dunstan wrote:
Well, we could put in similar weasel words I guess. But after
all, Safe's very purpose is to provide a restricted execution
environment, no?
We already do, in our license.
True. I think the weasel formula I prefer here is a bit different.
It might be reasonable to say something along the lines of:
To the extent it is prevented by the Perl Safe module, there is no
way provided to access internals of the database server process or
to gain OS-level access with the permissions of the server process,
as a C function can do.
Here's a patch that:
1. adds wording like that to the docs.
2. randomises the container package name (a simple and sound security measure).
3. requires Safe 2.25 (which has assorted fixes, including security).
4. removed a harmless but suprious exclamation mark from the source.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index c000463..0cc59c5 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** $$ LANGUAGE plperl;
*** 856,862
operations that are restricted are those that interact with the
environment. This includes file handle operations,
literalrequire/literal, and literaluse/literal (for
!external modules). There is no way to access internals of the
database server process or to gain OS-level access with the
permissions of the server process,
as a C function can do. Thus, any unprivileged database user can
--- 856,864
operations that are restricted are those that interact with the
environment. This includes file handle operations,
literalrequire/literal, and literaluse/literal (for
!external modules). To the extent it is prevented by the Perl
!ulink url=http://search.cpan.org/perldoc?Safe;Safe/ulink module,
!there is no way provided to access internals of the
database server process or to gain OS-level access with the
permissions of the server process,
as a C function can do. Thus, any unprivileged database user can
diff --git a/src/pl/plperl/plc_safe_ok.pl b/src/pl/plperl/plc_safe_ok.pl
index ee2e33f..873143f 100644
*** a/src/pl/plperl/plc_safe_ok.pl
--- b/src/pl/plperl/plc_safe_ok.pl
*** if (not our $_init++) {
*** 52,58
# --- create and initialize a new container ---
$SafeClass ||= 'Safe';
! $PLContainer = $SafeClass-new('PostgreSQL::InServer::safe_container');
$PLContainer-permit_only(':default');
$PLContainer-permit(qw[:base_math !:base_io sort time require]);
--- 52,64
# --- create and initialize a new container ---
$SafeClass ||= 'Safe';
! # Give the container a random name to complicate an attack that needs the name
! # (Iff perl is loaded via shared_preload_libraries and perl uses the same
! # random function as postgres then perl's own seed function would have already
! # been called and an attacker could call the postgres setseed() before first
! # use of plperl to control the rand result. Even so, we try to make life hard.)
! # There's no known exploit based on this but it's cheap and wise.
! $PLContainer = $SafeClass-new('PostgreSQL::InServer::safe'.int(rand(time+$^T+$!)));
$PLContainer-permit_only(':default');
$PLContainer-permit(qw[:base_math !:base_io sort time require]);
*** sub safe_eval {
*** 91,95
}
sub mksafefunc {
! ! return safe_eval(PostgreSQL::InServer::mkfuncsrc(@_));
}
--- 97,101
}
sub mksafefunc {
! return safe_eval(PostgreSQL::InServer::mkfuncsrc(@_));
}
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 956eddb..a834063 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*** plperl_trusted_init(void)
*** 691,702
safe_version_x100 = (int) (SvNV(safe_version_sv) * 100);
/*
! * Reject too-old versions of Safe and some others: 2.20:
! * http://rt.perl.org/rt3/Ticket/Display.html?id=72068 2.21:
! * http://rt.perl.org/rt3/Ticket/Display.html?id=72700
*/
! if (safe_version_x100 209 || safe_version_x100 == 220 ||
! safe_version_x100 == 221)
{
/* not safe, so disallow all trusted funcs */
eval_pv(PLC_SAFE_BAD, FALSE);
--- 691,699
safe_version_x100 = (int) (SvNV(safe_version_sv) * 100);
/*
! * Reject too-old versions of Safe
*/
! if (safe_version_x100 225)
{
/* not safe, so disallow all trusted funcs */
eval_pv(PLC_SAFE_BAD, FALSE);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Safe security
On Mon, Mar 08, 2010 at 11:03:27AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: Here's a patch that: 1. adds wording like that to the docs. 2. randomises the container package name (a simple and sound security measure). 3. requires Safe 2.25 (which has assorted fixes, including security). 4. removed a harmless but suprious exclamation mark from the source. #3 is still an absolute nonstarter, especially for a patch that we'd wish to backpatch. This is a patch for 9.0. Backpatching is a separate issue. I think Safe 2.25 should be required, but I'll let whoever applies the patch tweak/delete that hunk as desired. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Core dump running PL/Perl installcheck with bleadperl [PATCH]
I encountered a core dump running PL/Perl installcheck with a very
recent git HEAD of PostgreSQL and a not quite so recent git HEAD of perl.
The cause is a subtle difference between SvTYPE(sv) == SVt_RV and
SvROK(sv). The former is checking a low-level implementation detail
while the later is directly checking does this sv contains a reference.
The attached patch fixes the problem by changing the SvTYPE check to use
SvROK instead. Although I only tripped over one case, the patch changes
all four uses of SvTYPE(sv) == SVt_RV. The remaining uses of SvTYPE are ok.
Tim.
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 956eddb..c1cc8ff 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*** plperl_modify_tuple(HV *hvTD, TriggerDat
*** 976,982
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_COLUMN),
errmsg($_TD-{new} does not exist)));
! if (!SvOK(*svp) || SvTYPE(*svp) != SVt_RV || SvTYPE(SvRV(*svp)) != SVt_PVHV)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg($_TD-{new} is not a hash reference)));
--- 976,982
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_COLUMN),
errmsg($_TD-{new} does not exist)));
! if (!SvOK(*svp) || !SvROK(*svp) || SvTYPE(SvRV(*svp)) != SVt_PVHV)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg($_TD-{new} is not a hash reference)));
*** plperl_func_handler(PG_FUNCTION_ARGS)
*** 1549,1555
* value is an error, except undef which means return an empty set.
*/
if (SvOK(perlret)
! SvTYPE(perlret) == SVt_RV
SvTYPE(SvRV(perlret)) == SVt_PVAV)
{
int i = 0;
--- 1549,1555
* value is an error, except undef which means return an empty set.
*/
if (SvOK(perlret)
! SvROK(perlret)
SvTYPE(SvRV(perlret)) == SVt_PVAV)
{
int i = 0;
*** plperl_func_handler(PG_FUNCTION_ARGS)
*** 1594,1600
AttInMetadata *attinmeta;
HeapTuple tup;
! if (!SvOK(perlret) || SvTYPE(perlret) != SVt_RV ||
SvTYPE(SvRV(perlret)) != SVt_PVHV)
{
ereport(ERROR,
--- 1594,1600
AttInMetadata *attinmeta;
HeapTuple tup;
! if (!SvOK(perlret) || !SvROK(perlret) ||
SvTYPE(SvRV(perlret)) != SVt_PVHV)
{
ereport(ERROR,
*** plperl_return_next(SV *sv)
*** 2218,2224
errmsg(cannot use return_next in a non-SETOF function)));
if (prodesc-fn_retistuple
! !(SvOK(sv) SvTYPE(sv) == SVt_RV SvTYPE(SvRV(sv)) == SVt_PVHV))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg(SETOF-composite-returning PL/Perl function
--- 2218,2224
errmsg(cannot use return_next in a non-SETOF function)));
if (prodesc-fn_retistuple
! !(SvOK(sv) SvROK(sv) SvTYPE(SvRV(sv)) == SVt_PVHV))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg(SETOF-composite-returning PL/Perl function
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Safe security (was: plperl _init settings)
On Tue, Mar 02, 2010 at 07:33:47PM -0500, Andrew Dunstan wrote: There appears to be some significant misunderstanding of what can be done effectively using the various *_init settings for plperl. In particular, some people have got an expectation that modules loaded in plperl.on_init will thereby be available for use in trusted plperl. I propose to add the following note to the docs: Preloading modules using plperl.on_init does not make them available for use by plperl. External perl modules can only be used in plperlu. Comments? Sounds good. FYI the maintainers of Safe are aware of (at least) two exploits which are being considered at the moment. You might want to soften the wording in http://developer.postgresql.org/pgdocs/postgres/plperl-trusted.html There is no way to ... is a stronger statement than can be justified. The docs for Safe http://search.cpan.org/~rgarcia/Safe-2.23/Safe.pm#WARNING say The authors make no warranty, implied or otherwise, about the suitability of this software for safety or security purposes. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Wed, Feb 17, 2010 at 10:30:03AM -0800, David E. Wheeler wrote: On Feb 17, 2010, at 4:28 AM, Tim Bunce wrote: Yes, but if it's a variadic function, I suspect that it won't often be called with the same number of args. So you'd potentially end up caching a lot of extra stuff that would never be used again. Potentially. Patches welcome! GitHub. ;-P http://github.com/timbunce/posgtresql-plperl-call Umm, perhaps F-funcname(@args), or PG-funcname(@args), or ... ? Anyone got any better suggestions? PG is good. Or maybe DB? On Thu, Feb 18, 2010 at 08:26:51AM +, Richard Huxton wrote: It's a module whose only use is embedded in a DB called PG - not sure those carry any extra info. It also treads on the toes of PG-not_a_function should such a beast be needed. I like F-funcname or FN-funcname myself. Thanks. I quite like FN. Anybody else want to express an opinion on the color if this bikeshed before I repaint it? Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CommitFest Status Summary - 2010-02-14
On Tue, Feb 16, 2010 at 02:25:00PM -0800, David E. Wheeler wrote: On Feb 16, 2010, at 2:19 PM, Tim Bunce wrote: p.s. One quick heads-up: David Wheeler has reported a possible issue with Safe 2.21. I hope to investigate that tomorrow. Actually it's 2.22. 2.21 is already dead. Oops, typo. At this stage I think the problem is that the call to utf8fix (that's made when the compartment is initialized) is failing due to the extra security in Safe 2.2x. If so, PostgreSQL 9.0 will be fine but 8.x and earlier will require a patch. I'll start a new thread when I have some concrete information. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CommitFest Status Summary - 2010-02-14
On Tue, Feb 16, 2010 at 05:22:27PM -0500, Robert Haas wrote: It's certainly been an interesting introduction to PostgreSQL development! Hopefully we haven't scared you off - your work is definitely very much appreciated (and I at least hope to see you back for 9.1)! Thanks Robert. That's nice to hear. I'd be happy to do more for 9.1 (subject to the ongoing generosity of my client http://TigerLead.com who are the ones to thank for my work on PostgreSQL). Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Tue, Feb 16, 2010 at 02:13:30PM -0800, David E. Wheeler wrote: On Feb 16, 2010, at 2:06 PM, Tim Bunce wrote: For varadic functions, separate plans are created and cached for each distinct number of arguments the function is called with. Why? It keeps the code simple and repeat calls fast. Yes, but if it's a variadic function, I suspect that it won't often be called with the same number of args. So you'd potentially end up caching a lot of extra stuff that would never be used again. Potentially. Patches welcome! So, is this on GitHub yet? That way I can submit patches. I've uploaded PostgreSQL-PLPerl-Call-1.003.tar.gz to CPAN with these changes. It's in git but not github yet. Maybe soonish. I saw. I think it might pay to heed Richard's suggestion not to use SP. Umm, perhaps F-funcname(@args), or PG-funcname(@args), or ... ? Anyone got any better suggestions? By the way, I think it needs some documentation explaining how to load it inside PL/Perl. I thought about that, and started to write it, but dropped it for now. I'll wait till my cunning plan to share code with the Safe compartment (aka PostgreSQL::PLPerl::Injector) is done then document how call() can be used in both plperlu and plperl. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GUC failure on exception
Did this ever get turned into a formal bug report with a number? Tim. On Thu, Jan 14, 2010 at 07:39:09PM -0500, Andrew Dunstan wrote: Tim Bunce just showed me the following oddity: andrew=# SET SESSION plperl.use_strict = on; SET andrew=# SHOW plperl.use_strict; plperl.use_strict --- on (1 row) andrew=# DO $$ elog(ERROR,error) $$ language plperl; ERROR: error at line 1. CONTEXT: PL/Perl anonymous code block andrew=# SHOW plperl.use_strict; plperl.use_strict --- off (1 row) Somehow we have lost the setting, because the first use of plperl, which called the plperl init code, failed. It appears that whatever rolls it back forgets to put the GUC setting back as it was, and now it's lost, which is pretty darn ugly. And you can now run code which fails the 'strict' tests. If anyone has a quick idea about how to fix that would be nice. Otherwise I'll try to delve into it as time permits. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Mon, Feb 15, 2010 at 02:58:47PM -0800, David E. Wheeler wrote:
On Feb 15, 2010, at 2:42 PM, Tim Bunce wrote:
I've not really looked the the DBD::Pg code much so this seemed like a
good excuse... It looks like the default is to call PQprepare() with
paramTypes Oid values of 0.
Yes, IIRC, 0 == unknown as far as the server is concerned. It just
tells the server to resolve it when it can.
An extra source of puzzlement is that the oid of the 'unknown' type is
705 not 0, and the unknown type isn't discussed in the docs (as far as I
could see).
http://developer.postgresql.org/pgdocs/postgres/libpq-exec.html says
If paramTypes is NULL, or any particular element in the array is zero,
the server assigns a data type to the parameter symbol in the same way
it would do for an untyped literal string.
Right, exactly.
But I don't know if that means it has the same semantics as using
'unknown' as a type to PL/Perl's spi_prepare(). The docs for
spi_prepare() don't mention if type parameters are optional or what
happens if they're omitted.
http://developer.postgresql.org/pgdocs/postgres/plperl-builtins.html#PLPERL-DATABASE
Same as in SQL PREPARE, I'm sure. Ultimately that's what's doing the work,
IIUC.
Looking at the code I see spi_prepare() maps the provided arg type names
to oids then calls SPI_prepare(). The docs for SPI_prepare() also don't
mention if the type parameters are optional or what happens if they're
omitted.
The docs for the int nargs parameter say number of input *parameters*
not number of parameters that Oid *argtypes describes
http://developer.postgresql.org/pgdocs/postgres/spi-spi-prepare.html
Guess I need to go and check the current behaviour... see below.
And like maybe a doc patch might be useful.
I would be great if someone who understood
I'm currently using:
my $placeholders = join ,, map { '$'.$_ } 1..$arity;
my $plan = spi_prepare(select * from $spname($placeholders),
@$arg_types) };
Ah, yeah, that's better, but I do think you should use quote_ident() on the
function name.
That would cause complications if included a schema name. I've opted to
specify that the name used in the signature should be in quoted form if
it needs quoting.
and it turns out that spi_prepare is happy to prepare a statement with
more placeholders than there are types provided.
Types or args?
These appear to be identical in behaviour:
spi_prepare(select * from foo($1,$2), 'unknown', 'unknown');
spi_prepare(select * from foo($1,$2), 'unknown')
spi_prepare(select * from foo($1,$2))
You can't specify a schema though, and the 'SP' is somewhat
artificial. Still, I'm coming round to the idea :)
What about `SP-schema::function_name()`?
Wouldn't work unless you'd installed an AUTOLOAD function into each
schema:: package that you wanted to use. (schema-SP::function_name()
could be made to work but that's just too bizzare :)
Agreed that SP is artificial, but there needs to be some kind of
handle for AUTOLOAD to wrap itself around. Maybe a singleton object
instead? (I was kind of thinking of SP as that, anyway:
use constant SP = 'PostgreSQL::PLPerl';
)
Something like that is probably best. I've made PostgreSQL::PLPerl::Call
export both call and SP where SP is a constant containing the name
of a class (PostgreSQL::PLPerl::Call::SP) that just has an AUTOLOAD.
I've attached the current docs and code.
Thanks for your help David!
Tim.
package PostgreSQL::PLPerl::Call;
=head1 NAME
PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from
PostgreSQL PL/Perl
=head1 SYNOPSIS
use PostgreSQL::PLPerl::Call;
Returning single-row single-column values:
$pi = call('pi'); # 3.14159265358979
$net = call('network(inet)', '192.168.1.5/24'); # '192.168.1.0/24';
$seqn = call('nextval(regclass)', $sequence_name);
$dims = call('array_dims(text[])', '{a,b,c}'); # '[1:3]'
# array arguments can be perl array references:
$ary = call('array_cat(int[], int[])', [1,2,3], [2,1]); # '{1,2,3,2,1}'
Returning multi-row single-column values:
@ary = call('generate_series(int,int)', 10, 15); # (10,11,12,13,14,15)
Returning single-row multi-column values:
# assuming create function func(int) returns table (r1 text, r2 int) ...
$row = call('func(int)', 42); # returns hash ref { r1=..., r2=... }
Returning multi-row multi-column values:
@rows = call('pg_get_keywords'); # ({...}, {...}, ...)
Alternative method-call syntax:
$pi = SP-pi();
$seqn = SP-nextval($sequence_name);
=head1 DESCRIPTION
The Ccall function provides a simple efficient way to call SQL functions
from PostgreSQL PL/Perl code.
The first parameter is a Isignature that specifies the name of the function
to call and, optionally, the types of the arguments.
Any further parameters are used as argument values for the function being
called.
=head2 Signature
The first parameter
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Tue, Feb 16, 2010 at 09:11:24AM -0800, David E. Wheeler wrote:
On Feb 16, 2010, at 4:08 AM, Tim Bunce wrote:
From the docs:
Immediately after the function name, in parenthesis, a comma separated list
of
type names can be given. For example:
'pi()'
'generate_series(int,int)'
'array_cat(int[], int[])'
'myschema.myfunc(date, float8)'
It could also just be 'pi', no?
Yes. A vestige from when the parens were still needed. Fixed.
Functions with Cvaradic arguments can be called with a fixed number of
arguments by repeating the type name in the signature the same number of
times.
I assume that type names can be omitted her, too, yes?
No, it seems not. You have to either repeat the type name the right number
of times, or use '...', which simply duplicates the type name for you
behind the scenes. I'll clarify that in the docs (and fix all the
places I spelt variadic wrong :)
$pi = SP-pi();
$seqn = SP-nextval($sequence_name);
Using this form you can't easily specify a schema name or argument types,
SP-schema.func() doesn't work. ($name=schema.func; SP-$name() works.)
and you can't call varadic functions.
Why not?
Using spi_prepare('select * from variadic_func($1)') the error is there
is no parameter $1. I suspect calls to varadic functions do need
correct nargs and type information given to the SPI_prepare call.
Also, I notice a few `==head`s. I think that's one too many =s.
Fixed. Thanks.
For varadic functions, separate plans are created and cached for each
distinct
number of arguments the function is called with.
Why?
It keeps the code simple and repeat calls fast.
Functions with a varadic argument can't be called with no values for that
argument. You'll get a function ... does not exist error. This appears
to be
a PostgreSQL limitation.
Hrm. Worth enquiring about.
I found it in the docs: A parameter marked VARIADIC matches *one* or
more occurrences of its element type.
http://www.postgresql.org/docs/8.4/interactive/xfunc-sql.html
So, is this on GitHub yet? That way I can submit patches.
I've uploaded PostgreSQL-PLPerl-Call-1.003.tar.gz to CPAN with these
changes. It's in git but not github yet. Maybe soonish.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CommitFest Status Summary - 2010-02-14
On Tue, Feb 16, 2010 at 04:42:29PM -0500, Andrew Dunstan wrote: Tim Bunce wrote: On Sun, Feb 14, 2010 at 10:14:28PM -0500, Andrew Dunstan wrote: Robert Haas wrote: We're down to 5 patches remaining, and 1 day remaining, so it's time to try to wrap things up. * Package namespace and Safe init cleanup for plperl. Andrew Dunstan is taking care of this one, I believe. I will get this in, with changes as discussed recently. Here's a small extra patch for your consideration. It addresses a couple of minor loose-ends in plperl: - move on_proc_exit() call to after the plperl_*_init() calls so on_proc_exit will only be called if plperl_*_init() succeeds (else there's a risk of on_proc_exit consuming all the exit hook slots) - don't allow use of Safe version 2.21 as that's broken for PL/Perl. I have committed all the plperl changes that were under discussion, including this, and the change to the log level of perl warnings. Thanks for all your work, Tim, you have certainly given plperl a huge booster shot. Thanks Andrew! And many thanks to you and the rest of the PostgreSQL developers for all your support/resistance/reviews along the way. The final changes are certainly better in many ways (though not all ;-) from my original patches. It's certainly been an interesting introduction to PostgreSQL development! Tim. p.s. One quick heads-up: David Wheeler has reported a possible issue with Safe 2.21. I hope to investigate that tomorrow. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Mon, Feb 15, 2010 at 07:31:14AM +, Richard Huxton wrote:
On 12/02/10 23:10, Tim Bunce wrote:
There was some discussion a few weeks ago about inter-stored-procedure
calling from PL/Perl.
I'd greatly appreciate any feedback.
Looks great.
Thanks!
PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from
PostgreSQL PL/Perl
I don't think you show an example with an explicit schema name being
used. Can't hurt to make it obvious.
Yes, good point. I've added one to the docs and tests. Thanks.
$seqn = call('nextval(regclass)', $sequence_name);
Is there any value in having a two-stage interface?
$seq_fn = get_call('nextval(regclass)');
$foo1 = $seq_fn-($seq1);
$foo2 = $seq_fn-($seq2);
I don't think there's significant performance value in that.
Perhaps it could be useful to be able to pre-curry a call and
then pass that code ref around, but you can do that trivially
already:
$nextval_fn = sub { call('nextval(regclass)', @_) };
$val = $nextval_fn-($seq1);
or
$nextfoo_fn = sub { call('nextval(regclass)', 'foo_seqn') };
$val = $nextfoo_fn-();
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Sat, Feb 13, 2010 at 02:25:48PM -0800, David E. Wheeler wrote:
On Feb 12, 2010, at 3:10 PM, Tim Bunce wrote:
I've appended the POD documentation and attached the (rough but working)
test script.
I plan to release the module to CPAN in the next week or so.
I'd greatly appreciate any feedback.
I like the idea overall, and anything that can simplify the interface is more
than welcome. However:
* I'd rather not have to specify a signature for a non-polymorphic function.
The signature doesn't just qualify the selection of the function,
it also ensures appropriate interpretation of the arguments.
I could allow call('foo', @args), which could be written call(foo = @args),
but what should that mean in terms of the underlying behaviour?
I think there are three practical options:
a) treat it the same as call('foo(unknown...)', @args)
b) treat it the same as call('foo(text...)', @args)
c) instead of using a cached prepared query, build an SQL statement
for every execution, which would naturally have to quote all values:
my $args = join ,, map { ::quote_nullable($_) } @_;
return ::spi_exec_query(select * from $spname($args));
I suspect there are subtle issues (that I'm unfamilar with) lurking here.
I'd appreciate someone with greater understanding spelling out the issues
and trade-offs in those options.
* I'd like to be able to use Perl code to call the functions as discussed
previously, something like:
my $count_sql = SP-tl_activity_stats_sql(
[ statistic = $stat, person_id = $pid ],
$debug
);
For a Polymorphic function, perhaps it could be something like:
my $count = SP-call(
tl_activity_stats_sql = [qw(text[] int)],
[ statistic = $stat, person_id = $pid ],
$debug
);
The advantage here is that I'm not writing functions inside strings,
Umm,
tl_activity_stats_sql = [qw(text[] int)]
seems to me longer and rather less visually appealing than
'tl_activity_stats_sql(text[], int)'
and only provide the signature when I need to disambiguate between
polymorphic variants.
Or need to qualify the type of the argument for some other reason, like
passing an array reference.
But perhaps we can agree on one of the options a/b/c above and then
this issue will be less relevant. It's not like you'd be saving much
typing:
call('tl_activity_stats_sql', @args)
call(tl_activity_stats_sql = @args)
SP-tl_activity_stats_sql(@args)
You could always add a trivial SP::AUTOLOAD wrapper function to your
plperl.on_init code :)
Anyway, That's just interface arguing. The overall idea is sound and
very much appreciated.
Thanks!
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CommitFest Status Summary - 2010-02-14
On Sun, Feb 14, 2010 at 10:14:28PM -0500, Andrew Dunstan wrote:
Robert Haas wrote:
We're down to 5 patches remaining, and 1 day remaining, so it's time
to try to wrap things up.
* Package namespace and Safe init cleanup for plperl. Andrew Dunstan
is taking care of this one, I believe.
I will get this in, with changes as discussed recently.
Here's a small extra patch for your consideration.
It addresses a couple of minor loose-ends in plperl:
- move on_proc_exit() call to after the plperl_*_init() calls
so on_proc_exit will only be called if plperl_*_init() succeeds
(else there's a risk of on_proc_exit consuming all the exit hook slots)
- don't allow use of Safe version 2.21 as that's broken for PL/Perl.
Tim.
commit d8c0d4e63c00606db95f95a9c8f2b7ccf3c819b3
Author: Tim Bunce tim.bu...@pobox.com
Date: Mon Feb 15 11:18:07 2010 +
Move on_proc_exit to after init (that may fail). Avoid Safe 2.21.
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index e950222..16d74a7 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -365,8 +365,6 @@ select_perl_context(bool trusted)
{
/* first actual use of a perl interpreter */
- on_proc_exit(plperl_fini, 0);
-
if (trusted)
{
plperl_trusted_init();
@@ -379,6 +377,10 @@ select_perl_context(bool trusted)
plperl_untrusted_interp = plperl_held_interp;
interp_state = INTERP_UNTRUSTED;
}
+
+ /* successfully initialized, so arrange for cleanup */
+ on_proc_exit(plperl_fini, 0);
+
}
else
{
@@ -685,8 +687,9 @@ plperl_trusted_init(void)
/*
* Reject too-old versions of Safe and some others:
* 2.20: http://rt.perl.org/rt3/Ticket/Display.html?id=72068
+ * 2.21: http://rt.perl.org/rt3/Ticket/Display.html?id=72700
*/
- if (safe_version_x100 209 || safe_version_x100 == 220)
+ if (safe_version_x100 209 || safe_version_x100 == 220 || safe_version_x100 == 221)
{
/* not safe, so disallow all trusted funcs */
eval_pv(PLC_SAFE_BAD, FALSE);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Mon, Feb 15, 2010 at 10:42:15AM +, Richard Huxton wrote:
On 15/02/10 10:32, Tim Bunce wrote:
On Mon, Feb 15, 2010 at 07:31:14AM +, Richard Huxton wrote:
Is there any value in having a two-stage interface?
$seq_fn = get_call('nextval(regclass)');
$foo1 = $seq_fn-($seq1);
$foo2 = $seq_fn-($seq2);
I don't think there's significant performance value in that.
Perhaps it could be useful to be able to pre-curry a call and
then pass that code ref around, but you can do that trivially
already:
$nextval_fn = sub { call('nextval(regclass)', @_) };
$val = $nextval_fn-($seq1);
or
$nextfoo_fn = sub { call('nextval(regclass)', 'foo_seqn') };
$val = $nextfoo_fn-();
Fair enough. Just wondered whether it was worth putting that on your
side of the interface. I'm forced to concede you probably have more
experience in database-related APIs than me :-)
I've actually very little experience with PostgreSQL! I'm happy to argue
each case on its merits and am certainly open to education and persuasion.
At the moment I don't see enough gain to warrant an additional API.
I am adding the some examples to the docs though. So thanks for that!
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Mon, Feb 15, 2010 at 10:51:14AM +, Tim Bunce wrote: On Sat, Feb 13, 2010 at 02:25:48PM -0800, David E. Wheeler wrote: On Feb 12, 2010, at 3:10 PM, Tim Bunce wrote: I've appended the POD documentation and attached the (rough but working) test script. I plan to release the module to CPAN in the next week or so. I'd greatly appreciate any feedback. I like the idea overall, and anything that can simplify the interface is more than welcome. However: * I'd rather not have to specify a signature for a non-polymorphic function. The signature doesn't just qualify the selection of the function, it also ensures appropriate interpretation of the arguments. Just to clarify that... I mean appropriate interpretation not only by PostgreSQL but also by the call() code knowing which arguments may need array encoding (without having to check them all on every call). The signature also makes it easy to refer to functions in other schemas. Something that a SP-func_name(...) style syntax wouldn't allow. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Mon, Feb 15, 2010 at 11:52:01AM -0800, David E. Wheeler wrote:
On Feb 15, 2010, at 2:51 AM, Tim Bunce wrote:
The signature doesn't just qualify the selection of the function,
it also ensures appropriate interpretation of the arguments.
I could allow call('foo', @args), which could be written call(foo = @args),
but what should that mean in terms of the underlying behaviour?
I think there are three practical options:
a) treat it the same as call('foo(unknown...)', @args)
I believe that's basically what psql does. It's certainly what DBD::Pg does.
I've not really looked the the DBD::Pg code much so this seemed like a
good excuse... It looks like the default is to call PQprepare() with
paramTypes Oid values of 0.
http://developer.postgresql.org/pgdocs/postgres/libpq-exec.html says
If paramTypes is NULL, or any particular element in the array is zero,
the server assigns a data type to the parameter symbol in the same way
it would do for an untyped literal string.
But I don't know if that means it has the same semantics as using
'unknown' as a type to PL/Perl's spi_prepare(). The docs for
spi_prepare() don't mention if type parameters are optional or what
happens if they're omitted.
http://developer.postgresql.org/pgdocs/postgres/plperl-builtins.html#PLPERL-DATABASE
Looking at the code I see spi_prepare() maps the provided arg type names
to oids then calls SPI_prepare(). The docs for SPI_prepare() also don't
mention if the type parameters are optional or what happens if they're omitted.
The docs for the int nargs parameter say number of input *parameters*
not number of parameters that Oid *argtypes describes
http://developer.postgresql.org/pgdocs/postgres/spi-spi-prepare.html
Guess I need to go and check the current behaviour... see below.
c) instead of using a cached prepared query, build an SQL statement
for every execution, which would naturally have to quote all values:
my $args = join ,, map { ::quote_nullable($_) } @_;
return ::spi_exec_query(select * from $spname($args));
I suspect there are subtle issues (that I'm unfamilar with) lurking here.
I'd appreciate someone with greater understanding spelling out the issues
and trade-offs in those options.
I'm pretty sure the implementation doesn't have to declare the types of
anything:
sub AUTOLOAD {
my $self = shift;
our $AUTOLOAD;
(my $fn = $AUTOLOAD) =~ s/.*://;
my $prepared = spi_prepare(
'EXECUTE ' . quote_ident($fn) . '('
. join(', ', ('?') x @_)
. ')';
# Cache it and call it.
}
I'm currently using:
my $placeholders = join ,, map { '$'.$_ } 1..$arity;
my $plan = spi_prepare(select * from $spname($placeholders), @$arg_types)
};
and it turns out that spi_prepare is happy to prepare a statement with
more placeholders than there are types provided.
I'm a little nervous of relying on that undocumented behaviour.
Hopefully someone can clarify if that's expected behaviour.
So, anyway, I've now extended the code so the parenthesis and types
aren't needed. Thanks for prompting the investigation :)
Umm,
tl_activity_stats_sql = [qw(text[] int)]
seems to me longer and rather less visually appealing than
'tl_activity_stats_sql(text[], int)'
That would work, too. But either way, having to specify the signature
would be the exception rather than the rule. You'd only need to do it
when calling a polymorphic function with the same number of arguments
as another polymorphic function.
[Tick]
and only provide the signature when I need to disambiguate between
polymorphic variants.
Or need to qualify the type of the argument for some other reason, like
passing an array reference.
I don't think it's necessary. I mean, if you're passed an array, you
should of course pass it to PostgreSQL, but it can be anyarray.
Sure, you can pass an array in encoded string form, no problem.
But specifying in the signature a type that includes [] enables
you to use a perl array _reference_ and let call() look after
encoding it for you.
I did it that way round, rather than checking all the args for refs on
every call, as it felt safer, more efficient, and more extensible.
But perhaps we can agree on one of the options a/b/c above and then
this issue will be less relevant. It's not like you'd be saving much
typing:
call('tl_activity_stats_sql', @args)
call(tl_activity_stats_sql = @args)
SP-tl_activity_stats_sql(@args)
No, but the latter is more Perlish.
True. You can't specify a schema though, and the 'SP' is somewhat
artificial. Still, I'm coming round to the idea :)
You could always add a trivial SP::AUTOLOAD wrapper function to your
plperl.on_init code :)
Yeah yeah. I could even put one on CPAN. ;-P
I think it only needs this (untested):
package SP;
sub AUTOLOAD { our $AUTOLOAD =~ s/^SP:://; shift; call($AUTOLOAD
Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]
On Fri, Feb 12, 2010 at 07:57:15PM -0500, Andrew Dunstan wrote: Alex Hunsaker wrote: Yes it could allow people who can set the plperl.*_init functions to muck with the safe. As an admin I could also do that by setting plperl.on_init and overloading subs in the Safe namespace or switching out Safe.pm. It's quite easy to subvert Safe.pm today, sadly. You don't need on*init, nor do you need to replace the System's Safe.pm. I'm not going to tell people how here, but it took me all of a few minutes to discover and verify when I went looking a few weeks ago, once I thought about it. But that's quite different from us providing an undocumented way to expose arbitrary objects to the Safe container. In that case *we* become responsible for any insecure uses, and we don't even have the luxury of having put large warnings in the docs, because there aren't any docs. I wish I understood better how PostgreSQL developers would be responsible for a DBA using an undocumented hack. In my view the DBA is clearly responsible in that case. If it's not documented it's not supported. Another objection is that discussion on this facility has been pretty scant. I think that's putting it mildly. Maybe it's been apparent to a few people what the implications are, but I doubt it's been apparent to many. That makes me quite nervous, which is why I'm raising it now. Tim mentioned in his reply that he'd be happy to put warnings in plc_safe_ok.pl. But that file only exists in the source - it's turned into header file data as part of the build process, so warnings there will be seen by roughly nobody. I still think if we do this at all it needs to be documented and surrounded with appropriate warnings. I'm away for a day or so (for my wedding anniversary) but I'll have an updated patch, with a clean well-documented API, for consideration by Monday morning. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Fri, Feb 12, 2010 at 12:22:28AM -0500, Robert Haas wrote: On Wed, Feb 3, 2010 at 6:41 PM, Andrew Dunstan and...@dunslane.net wrote: Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: %_SHARED has been around for several years now, and if there are genuine security concerns about it ISTM they would apply today, regardless of these patches. Yes. I am not at all happy about inserting nonstandard permissions checks into GUC assign hooks --- they are not really meant for that and I think there could be unexpected consequences. Without a serious demonstration of a real problem that didn't exist before, I'm not in favor of it. Agreed. I think a more reasonable answer is just to add a documentation note pointing out that %_SHARED should be considered insecure in a multi-user database. Seems fair. What I was actually wondering about, however, is the extent to which the semantics of Perl code could be changed from an on_init hook --- is there any equivalent of changing search_path or otherwise creating trojan-horse code that might be executed unexpectedly? And if so is there any point in trying to guard against it? AIUI there isn't anything that can be done in on_init that couldn't be done in somebody else's function anyhow. The user won't be able to do anything in the on_init hook that they could not do from a plperl function anyway. In fact less, as SPI is not being made available. Plperl functions can't load arbitrary modules at all, and can't modify perl's equivalent of search_path. So I think the plain answer to your wonder ins no, it can't happen. There has been discussion in the past about allowing plperl functions access to certain modules. There is some sense in doing this, as it would help to avoid having to write plperlu for perfectly innocuous operations. But the list of allowed modules would have to be carefully controlled in something like a SIGHUP setting. We're certainly not going to allow such decisions to be made by anyone but the DBA. The discussion on this seems to have died off. Andrew, do you have something you're planning to commit for this? I think we're kind of down to the level of bikeshedding here... Following this thread I posted an updated patch: http://archives.postgresql.org/message-id/20100205134044.go52...@timac.local which Alex Hunsaker reviewed (and marked Ready For Committer) in http://archives.postgresql.org/pgsql-hackers/2010-02/msg00391.php Andrew Dunstan also reviewed it and highlighted some docs issues in http://archives.postgresql.org/pgsql-hackers/2010-02/msg00488.php I replied in http://archives.postgresql.org/pgsql-hackers/2010-02/msg00515.php and posted a further patch update to reflect the needed doc changes in http://archives.postgresql.org/message-id/20100208204213.gh1...@timac.local That updated patch is in the commitfest https://commitfest.postgresql.org/action/patch_view?id=271 From talking to Andrew via IM I understand he's very busy, and also that he'll be traveling on Sunday and Monday. I certainly hope he can commit this patch today (along with the next patch, which is also marked ready for committer) but if not, is there someone else who could? What happens to patches marked 'ready for committer' after the commitfest ends? Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
There was some discussion a few weeks ago about inter-stored-procedure
calling from PL/Perl.
I thought I'd post the documentation (and tests) for a module I'm
working on to simplify calling SQL functions from PL/Perl.
Here are some real-world examples (not the best code, but genuine
use-cases):
Calling a function that returns a single value (single column):
Old:
$count_sql = spi_exec_query(SELECT * FROM tl_activity_stats_sql('
. $to_array(statistic= $stat, person_id = $lead-{person_id})
. '::text[], $debug))-{rows}-[0]-{tl_activity_stats_sql};
New:
$count_sql = call('tl_activity_stats_sql(text[],int)',
[ statistic= $stat, person_id = $lead-{person_id} ], $debug);
The call() function recognizes the [] in the signature and knows that it
needs to handle the corresponding argument being an array reference.
Calling a function that returns a single record (multiple columns):
Old:
$stat_sql = SELECT * FROM tl_priority_stats($lead-{id}, $debug);
$stat_sth = spi_query($stat_sql);
$stats = spi_fetchrow($stat_sth);
New:
$stats = call('tl_priority_stats(int,int)', $lead-{id}, $debug);
Calling a function that returns multiple rows of a single value:
Old:
my $sql = SELECT * FROM tl_domain_mlx_area_ids($mlx_board_id, $domain_id,
$debug);
my $sth = spi_query($sql);
while( my $row = spi_fetchrow($sth) ) {
push(@mlx_area_ids, $row-{tl_domain_mlx_area_ids});
}
New:
@mlx_area_ids = call('tl_domain_mlx_area_ids(int,int,int)', $mlx_board_id,
$domain_id, $debug);
I've appended the POD documentation and attached the (rough but working)
test script.
I plan to release the module to CPAN in the next week or so.
I'd greatly appreciate any feedback.
Tim.
=head1 NAME
PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from
PostgreSQL PL/Perl
=head1 SYNOPSIS
use PostgreSQL::PLPerl::Call qw(call);
Returning single-row single-column values:
$pi = call('pi()'); # 3.14159265358979
$net = call('network(inet)', '192.168.1.5/24'); # '192.168.1.0/24';
$seqn = call('nextval(regclass)', $sequence_name);
$dims = call('array_dims(text[])', '{a,b,c}'); # '[1:3]'
# array arguments can be perl array references:
$ary = call('array_cat(int[], int[])', [1,2,3], [2,1]); # '{1,2,3,2,1}'
Returning multi-row single-column values:
@ary = call('generate_series(int,int)', 10, 15); # (10,11,12,13,14,15)
Returning single-row multi-column values:
# assuming create function func(int) returns table (r1 text, r2 int) ...
$row = call('func(int)', 42); # returns hash ref { r1=..., r2=... }
Returning multi-row multi-column values:
@rows = call('pg_get_keywords()'); # ({...}, {...}, ...)
=head1 DESCRIPTION
The Ccall function provides a simple effcicient way to call SQL functions
from PostgreSQL PL/Perl code.
The first parameter is a Isignature that specifies the name of the function
to call and then, in parenthesis, the types of any arguments as a comma
separated list. For example:
'pi()'
'generate_series(int,int)'
'array_cat(int[], int[])'
The types specify how the Iarguments to the call should be interpreted.
They don't have to exactly match the types used to declare the function you're
calling.
Any further parameters are used as arguments to the function being called.
=head2 Array Arguments
The argument value corresponding to a type that contains 'C[]' can be a
string formated as an array literal, or a reference to a perl array. In the
later case the array reference is automatically converted into an array literal
using the Cencode_array_literal() function.
=head2 Varadic Functions
Functions with Cvaradic arguments can be called with a fixed number of
arguments by repeating the type name in the signature the same number of times.
For example, given:
create function vary(VARADIC int[]) as ...
you can call that function with three arguments using:
call('vary(int,int,int)', $int1, $int2, $int3);
Alternatively, you can append the string 'C...' to the last type in the
signature to indicate that the argument is varadic. For example:
call('vary(int...)', @ints);
=head2 Results
The Ccall() function processes return values in one of four ways depending on
two criteria: single column vs. multi-column results, and list context vs
scalar context.
If the results contain a single column with the same name as the function that
was called, then those values are extracted returned directly. This makes
simple calls very simple:
@ary = call('generate_series(int,int)', 10, 15); # (10,11,12,13,14,15)
Otherwise, the rows are returned as references to hashes:
@rows = call('pg_get_keywords()'); # ({...}, {...}, ...)
If the Ccall() function was executed in list context then all the values/rows
are returned, as shown above.
If the function was executed in scalar context then an exception will be thrown
if more than one row is returned. For example:
$foo =
[HACKERS] Re: PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl
On Fri, Feb 12, 2010 at 11:10:15PM +, Tim Bunce wrote:
I've appended the POD documentation and attached the (rough but working)
test script.
Oops. Here's the test script.
Tim.
create or replace function test_call() returns text language plperlu as $func$
use lib /Users/timbo/pg/PostgreSQL-PLPerl-Call/lib;
use PostgreSQL::PLPerl::Call;
use Test::More 'no_plan';
my $row;
my @ary;
# == single-value single-row function ==
# --- no arguments
like call('pi()'), qr/^3.14159/;
# bad calls
eval { call('pi()', 42) };
like $@, qr/expected 0 argument/;
eval { call('pi', 42) };
like $@, qr/Can't parse 'pi'/; # error from call() itself
# --- one argument, simple types
is call('abs(int)', -42), 42;
is call('abs(float)', -42.5), '42.5';
is call('bit_length(text)', 'jose'), 32;
# --- one argument, multi-word types
is call('abs(double precision)', -42.5), '42.5';
is call('bit_length(character varying(90))', 'jose'), 32;
# --- lock calls
call('pg_try_advisory_lock_shared(bigint)', 1234);
call('pg_advisory_unlock_all()');
# bad calls
eval { call('abs(int)', -42.5) };
like $@, qr/invalid input syntax for integer/;
eval { call('abs(text)', -42.5) };
like $@, qr/function abs\(text\) does not exist/;
eval { call('abs(nonesuchtype)', -42.5) };
like $@, qr/type nonesuchtype does not exist/;
# --- multi-argument, simple types
is call('trunc(numeric,int)', 42.4382, 2), '42.43';
# --- unusual types from strings
is call('host(inet)','192.168.1.5/24'), '192.168.1.5';
is call('network(inet)', '192.168.1.5/24'), '192.168.1.0/24';
is call('abbrev(cidr)', '10.1.0.0/16'),'10.1/16';
is call('numnode(tsquery)', '(fat rat) | cat'), 5;
spi_exec_query('create temp sequence seqn1 start with 42');
is call('nextval(regclass)', 'seqn1'), 42;
is call('nextval(text)', 'seqn1'), 43;
is call('string_to_array(text, text)', 'xx~^~yy~^~zz', '~^~'), '{xx,yy,zz}';
# --- array and array reference handling
is call('array_dims(text[])', '{a,b,c}'), '[1:3]';
is call('array_dims(text[])', [qw(a b c)]), '[1:3]';
is call('array_dims(text[])', [[1,2,3], [4,5,6]]), '[1:2][1:3]';
is call('array_cat(int[], int[])', [1,2,3], [2,1]), '{1,2,3,2,1}';
# == single-value multi-row function ==
@ary = call('unnest(int[])', '{11,12,13}');
is scalar @ary, 3;
is_deeply \...@ary, [ 11, 12, 13 ];
@ary = call('generate_series(int,int)', 10, 19);
is scalar @ary, 10;
is_deeply \...@ary, [ 10..19 ];
@ary = call('generate_series(int,int,int)', 10, 19, 4);
is_deeply \...@ary, [ 10, 14, 18 ];
@ary = call('generate_series(timestamp,timestamp,interval)', '2008-03-01',
'2008-03-02', '12 hours');
is_deeply \...@ary, [ '2008-03-01 00:00:00', '2008-03-01 12:00:00', '2008-03-02
00:00:00' ];
# bad calls
eval { scalar call('generate_series(int,int)', 10, 19) };
like $@, qr/returned more than one row/;
# == multi-value (record) returning functions ==
@ary = call('pg_get_keywords()');
cmp_ok scalar @ary, '', 200;
ok $row = $ary[0];
is ref $row, 'HASH';
ok exists $row-{word},'should contain a word column';
ok exists $row-{catcode}, 'should contain a catcode column';
ok exists $row-{catdesc}, 'should contain a catdesc column';
# single-record
spi_exec_query(q{
create or replace function f1(out r1 text, out r2 int) language plperl
as $$
return { r1=10, r2=11 };
$$
});
@ary = call('f1()');
is scalar @ary, 1;
ok $row = $ary[0];
is $row-{r1}, 10;
is $row-{r2}, 11;
spi_exec_query('drop function f1()');
# multi-record
spi_exec_query(q{
create or replace function f2() returns table (r1 text, r2 int)
language plperl as $$
return_next { r1 = $_, r2 = $_+1 } for 1..5;
return undef;
$$
});
@ary = call('f2()');
is scalar @ary, 5;
is $ary[-1]-{r1}, 5;
is $ary[-1]-{r2}, 6;
spi_exec_query('drop function f2()');
# == functions with defaults or varargs ==
spi_exec_query(q{
create or replace function f3(int default 42) returns int language
plperl as $$
return shift() + 1;
$$
});
is call('f3()'), 43;
spi_exec_query('drop function f3(int)');
spi_exec_query(q{
create or replace function f4(VARIADIC numeric[]) returns float
language plperlu as $$
use PostgreSQL::PLPerl::Call;
my $sum;
$sum += $_ for call('unnest(numeric[])', $_[0]);
return $sum;
$$
});
# call varadic with explicit number of args in the signature
is call('f4(numeric, numeric)', 10,11 ), 21;
is call('f4(numeric, numeric, numeric)', 10,11,12), 33;
# call varadic using '...' in the signature
is call('f4(numeric, numeric ...)', 10,11,12), 33;
is call('f4(numeric ...)', 10,11,12), 33;
is call('f4(numeric ...)', 10,11 ), 21;
is call('f4(numeric ...)', 10 ), 10;
# XXX doesn't work with no args - possibly natural consequence
#is call('f4(numeric ...)' ), undef;
spi_exec_query('drop function f4(varadic
Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]
On Fri, Feb 12, 2010 at 02:30:37PM -0700, Alex Hunsaker wrote: On Fri, Feb 12, 2010 at 13:42, Andrew Dunstan and...@dunslane.net wrote: Alex Hunsaker wrote: and leave the rest for the next release, unless you can convince me real fast that we're not opening a back door here. If we're going to allow DBAs to add things to the Safe container, it's going to be up front or not at all, as far as I'm concerned. I think backdoor is a bit extreme. Yes it could allow people who can set the plperl.*_init functions to muck with the safe. As an admin I could also do that by setting plperl.on_init and overloading subs in the Safe namespace or switching out Safe.pm. Exactly. [As I mentioned before, the docs for Devel::NYTProf::PgPLPerl http://code.google.com/p/perl-devel-nytprof/source/browse/trunk/lib/Devel/NYTProf/PgPLPerl.pm talk about the need to _hack_ perl standard library modules in order to be able to run NYTProf on plperl code. The PERL5OPT environment variable could be used as an alternative vector.] Is there any kind of threat model (http://en.wikipedia.org/wiki/Threat_model) that PostgreSQL developers use when making design decisions? Clearly the PostgreSQL developers take security very seriously, and rightly so. An explicit threat model document would provide a solid framework to assess potential security issues when their raised. The key issue here seems to be the trust, or lack of it, placed in DBAs. Anyway reasons I can come up for it are: -its general so we can add other modules/pragmas easily in the future -helps with the plperl/plperlu all or nothing situation -starts to flesh out how an actual exposed (read documented) interface should look for 9.1 ... I know Tim mentioned David as having some use cases (cc'ed) I've just posted the docs for a module that's a good example of the kind of module a DBA might want to allow plperl code to use http://archives.postgresql.org/pgsql-hackers/2010-02/msg01059.php I'd be happy to add a large scary warning in the plc_safe_ok.pl file saying that any manipulation of the (undocumented) variables could cause a security risk and is not supported in any way. Tim. So my $0.2 is I don't have any strong feelings for/against it. I think if we could expose *something* (even if you can only get to it in a C contrib module) that would be great. But I realize it might be impractical at this stage in the game. *shrug* -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]
On Sun, Feb 07, 2010 at 08:25:33PM -0500, Andrew Dunstan wrote: Tim Bunce wrote: This is the third update to the fourth of the patches to be split out from the former 'plperl feature patch 1'. Changes in this patch: - Added plperl.on_plperl_init and plperl.on_plperlu_init GUCs Both are PGC_SUSET SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) Corresponding documentation and tests for both. - Renamed plperl.on_perl_init to plperl.on_init - Improved state management of select_perl_context() An error during interpreter initialization will leave the state (interp_state etc) unchanged. - The utf8fix code has been greatly simplified. - More code comments re PGC_SUSET and no access to SPI functions. The docs on this patch need some cleaning up and expanding: * The possible insecurity of %_SHARED needs expanding. I tried. I couldn't decide how to expand what Tom Lane suggested (http://archives.postgresql.org/message-id/1344.1265223...@sss.pgh.pa.us) without it turning into a sprawling security tutorial. So, since his suggestion seemed complete enough (albeit fairly vague), I just used it almost verbatim. Also, the PL/Tcl docs don't mention the issue at all and the PL/Python docs say only The global dictionary GD is public data, available to all Python functions within a session. Use with care. The wording in the PL/Python docs seems better (available to all ... use with care), so I've adopted the same kind of wording. * The docs still refer to plperl.on_untrusted_init Oops. Thanks. Fixed. * plperl.on_plperl_init and plperl.on_plperlu_init can be documented together rather than repeating the same stuff almost word for word. Ok. Done. * extra examples for these two, and an explanation of why one might want to use each of the three on*init settings would be good. plperl.on_init has an example that seems fairly self-explantory. I've added one to the on_plperl_init/on_plperlu_init section that also explains how a superuser can use ALTER USER ... SET to set a value for a non-superuser. I'll continue reviewing the patch, but these things at least need fixing. I've an updated patch ready to go. I'll hold on to it for now. Let me know if you have any more issues, or not. Thanks. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]
On Mon, Feb 08, 2010 at 01:44:16PM +, Tim Bunce wrote: I'll continue reviewing the patch, but these things at least need fixing. Here's an updated patch. The only changes relative to the previous version are in the docs, as I outlined in the previous message. I'll add it to the commitfest. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]
[Sigh. This email, unlike the previous, actually includes the patch.]
On Mon, Feb 08, 2010 at 01:44:16PM +, Tim Bunce wrote:
I'll continue reviewing the patch, but these things at least need fixing.
Here's an updated patch. The only changes relative to the previous
version are in the docs, as I outlined in the previous message.
I'll add it to the commitfest.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 7018624..f742f0b 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** $$ LANGUAGE plperl;
*** 748,753
--- 748,759
literalreturn $_SHARED{myquote}-gt;($_[0]);/literal
at the expense of readability.)
/para
+
+ para
+ The varname%_SHARED/varname variable and other global state within
+ the language is public data, available to all PL/Perl functions within a
+ session. Use with care.
+ /para
/sect1
sect1 id=plperl-trusted
*** CREATE TRIGGER test_valid_id_trig
*** 1044,1069
variablelist
! varlistentry id=guc-plperl-on-perl-init xreflabel=plperl.on_perl_init
! termvarnameplperl.on_perl_init/varname (typestring/type)/term
indexterm
!primaryvarnameplperl.on_perl_init/ configuration parameter/primary
/indexterm
listitem
para
!Specifies perl code to be executed when a perl interpreter is first initialized.
The SPI functions are not available when this code is executed.
If the code fails with an error it will abort the initialization of the interpreter
and propagate out to the calling query, causing the current transaction
or subtransaction to be aborted.
/para
para
!The perl code is limited to a single string. Longer code can be placed
!into a module and loaded by the literalon_perl_init/ string.
Examples:
programlisting
! plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;'
/programlisting
/para
para
--- 1050,1076
variablelist
! varlistentry id=guc-plperl-on-init xreflabel=plperl.on_init
! termvarnameplperl.on_init/varname (typestring/type)/term
indexterm
!primaryvarnameplperl.on_init/ configuration parameter/primary
/indexterm
listitem
para
!Specifies Perl code to be executed when a Perl interpreter is first initialized
!and before it is specialized for use by literalplperl/ or literalplperlu/.
The SPI functions are not available when this code is executed.
If the code fails with an error it will abort the initialization of the interpreter
and propagate out to the calling query, causing the current transaction
or subtransaction to be aborted.
/para
para
!The Perl code is limited to a single string. Longer code can be placed
!into a module and loaded by the literalon_init/ string.
Examples:
programlisting
! plplerl.on_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_init = 'use lib /my/app; use MyApp::PgInit;'
/programlisting
/para
para
*** plplerl.on_perl_init = 'use lib /my/app
*** 1077,1082
--- 1084,1129
/listitem
/varlistentry
+ varlistentry id=guc-plperl-on-plperl-init xreflabel=plperl.on_plperl_init
+ termvarnameplperl.on_plperl_init/varname (typestring/type)/term
+ termvarnameplperl.on_plperlu_init/varname (typestring/type)/term
+ indexterm
+primaryvarnameplperl.on_plperl_init/ configuration parameter/primary
+ /indexterm
+ indexterm
+primaryvarnameplperl.on_plperlu_init/ configuration parameter/primary
+ /indexterm
+ listitem
+para
+These parameters specify Perl code to be executed when the
+literalplperl/, or literalplperlu/ language is first used in a
+session. Changes to these parameters after the corresponding language
+has been used will have no effect.
+The SPI functions are not available when this code is executed.
+Only superusers can change these settings.
+The Perl code in literalplperl.on_plperl_init/ can only perform trusted operations.
+/para
+para
+The effect of setting these parameters is very similar to executing a
+literalDO/ command with the Perl code before any other use of the
+language. The parameters are useful when you want to execute the Perl
+code automatically on every connection, or when a connection is not
+interactive. The parameters can be used by non-superusers by having a
+superuser execute an literalALTER USER ... SET .../ command.
+For example:
+ programlisting
+ ALTER USER joe SET plplerl.on_plperl_init = '$_SHARED{debug} = 1
Re: [HACKERS] damage control mode
On Sat, Feb 06, 2010 at 10:38:00PM -0800, Josh Berkus wrote: Add on_trusted_init and on_untrusted_init to plperl Package namespace and Safe init cleanup for plperl Alex Hunsaker has marked the latest version of both of those as Ready for Committer. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]
This is the third update to the fourth of the patches to be split out
from the former 'plperl feature patch 1'.
Changes in this patch:
- Added plperl.on_plperl_init and plperl.on_plperlu_init GUCs
Both are PGC_SUSET
SPI functions are not available when the code is run.
Errors are detected and reported as ereport(ERROR, ...)
Corresponding documentation and tests for both.
- Renamed plperl.on_perl_init to plperl.on_init
- Improved state management of select_perl_context()
An error during interpreter initialization will leave
the state (interp_state etc) unchanged.
- The utf8fix code has been greatly simplified.
- More code comments re PGC_SUSET and no access to SPI functions.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 7018624..0999bd0 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** $$ LANGUAGE plperl;
*** 748,753
--- 748,758
literalreturn $_SHARED{myquote}-gt;($_[0]);/literal
at the expense of readability.)
/para
+
+ para
+ The varname%_SHARED/varname variable, and other global state within
+ the language, should be considered insecure in a multi-user database.
+ /para
/sect1
sect1 id=plperl-trusted
*** CREATE TRIGGER test_valid_id_trig
*** 1044,1057
variablelist
! varlistentry id=guc-plperl-on-perl-init xreflabel=plperl.on_perl_init
! termvarnameplperl.on_perl_init/varname (typestring/type)/term
indexterm
!primaryvarnameplperl.on_perl_init/ configuration parameter/primary
/indexterm
listitem
para
!Specifies perl code to be executed when a perl interpreter is first initialized.
The SPI functions are not available when this code is executed.
If the code fails with an error it will abort the initialization of the interpreter
and propagate out to the calling query, causing the current transaction
--- 1049,1063
variablelist
! varlistentry id=guc-plperl-on-init xreflabel=plperl.on_init
! termvarnameplperl.on_init/varname (typestring/type)/term
indexterm
!primaryvarnameplperl.on_init/ configuration parameter/primary
/indexterm
listitem
para
!Specifies perl code to be executed when a perl interpreter is first initialized
!and before it is specialized for use by literalplperl/ or literalplperlu/.
The SPI functions are not available when this code is executed.
If the code fails with an error it will abort the initialization of the interpreter
and propagate out to the calling query, causing the current transaction
*** CREATE TRIGGER test_valid_id_trig
*** 1059,1069
/para
para
The perl code is limited to a single string. Longer code can be placed
!into a module and loaded by the literalon_perl_init/ string.
Examples:
programlisting
! plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;'
/programlisting
/para
para
--- 1065,1075
/para
para
The perl code is limited to a single string. Longer code can be placed
!into a module and loaded by the literalon_init/ string.
Examples:
programlisting
! plplerl.on_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_init = 'use lib /my/app; use MyApp::PgInit;'
/programlisting
/para
para
*** plplerl.on_perl_init = 'use lib /my/app
*** 1077,1082
--- 1083,1134
/listitem
/varlistentry
+ varlistentry id=guc-plperl-on-plperl-init xreflabel=plperl.on_plperl_init
+ termvarnameplperl.on_plperl_init/varname (typestring/type)/term
+ indexterm
+primaryvarnameplperl.on_plperl_init/ configuration parameter/primary
+ /indexterm
+ listitem
+para
+Specifies perl code to be executed when the literalplperl/ language
+is first used in a session. Changes made after the literalplperl/
+language has been used will have no effect.
+The perl code can only perform trusted operations.
+The SPI functions are not available when this code is executed.
+/para
+para
+If the code fails with an error it will abort the initialization and
+propagate out to the calling query, causing the current transaction or
+subtransaction to be aborted. Any changes within perl won't be undone.
+If the literalplperl/ language is used again the
+initialization will be repeated.
+/para
+ /listitem
+ /varlistentry
+
+ varlistentry id=guc-plperl-on-untrusted-init xreflabel=plperl.on_untrusted_init
+ termvarnameplperl.on_untrusted_init/varname (typestring/type)/term
+ indexterm
+
Re: [HACKERS] Confusion over Python drivers
On Fri, Feb 05, 2010 at 09:19:26AM -0500, Bruce Momjian wrote: My son has brought to my attention that our current crop of Python client libraries is inadequate/confusing. I took a look myself, and asked on our IRC channel, and am now convinced this area needs attention. http://wiki.postgresql.org/wiki/Python The Python-hosted PostgreSQL page has similar problems: http://wiki.python.org/moin/PostgreSQL Does Perl have a similar mess? I don't think so. The primary database interface is DBI and as far as I can see there's only one DBI PostgreSQL driver: http://search.cpan.org/dist/DBD-Pg/ The only non-DBI interfaces I could find (by skimming the 384 results for postgresql on search.cpan.org) were: Postgres: http://search.cpan.org/dist/Postgres/last updated in 1998. Pg: http://search.cpan.org/dist/pgsql_perl5/ last updated in 2000. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Tue, Feb 02, 2010 at 08:42:21PM -0700, Alex Hunsaker wrote: On Sat, Jan 30, 2010 at 08:49, Tim Bunce tim.bu...@pobox.com wrote: SPI functions are not available when the code is run. Hrm, we might want to stick why in the docs or as a comment somewhere. I think this was the main concern? * We call a plperl function for the first time in a session, causing plperl.so to be loaded. This happens in the context of a superuser calling a non-superuser security definer function, or perhaps vice versa. Whose permissions apply to whatever the on_load code tries to do? (Hint: every answer is wrong.) It's hard to convey the underlying issues in a sentence or two. I tried. I'm not sure it's worth it. (Though if anyone thinks it is I'd be happy to get some specific suggestions for the wording to use.) - The utf8fix code has been greatly simplified. Yeah to the point that it makes me wonder if the old code had some reason to spin up the FunctionCall stuff. Do you happen to know? Before my refactoring led me to add safe_eval(), FunctionCall was 'the natural way' to invoke code inside the Safe compartment. The tests dont seem to pass :( this is from a make installcheck-world + ERROR: unrecognized configuration parameter plperl.on_trusted_init If I throw a LOAD 'plperl'; at the top of those sql files it works... Ah. That's be because I've got custom_variable_classes = 'plperl' in my postgresql.conf. I'll add LOAD 'plperl'; to the top of those tests. The only quibble I have with the docs is: + If the code fails with an error it will abort the initialization and + propagate out to the calling query, causing the current transaction or + subtransaction to be aborted. Any changes within the perl won't be + undone. If the literalplperl/ language is used again the + initialization will be repeated. Instead of Any changes within the perl won't be undone. Maybe Changes to the perl interpreter will not be undone ? In an earlier patch I'd used the word interpreter quite often. When polishing up the doc changes in response to Tom's feedback I opted to avoid that word when describing on_*trusted_init. This looks like a case where I removed 'interpreter' but didn't fixup the rest of the sentence. I'd prefer to simplify the sentence further, so I've changed it to Any changes within perl won't be undone. On Wed, Feb 03, 2010 at 01:06:03AM -0700, Alex Hunsaker wrote: plperl.on_trusted_init runs *inside* of the safe. So you cant do unsafe things like use this or that module. Yes. It's effectively the same as having a DO '...' language plperl; that's guaranteed to run before any other use of plperl. plperl.on_init runs on init *outside* of the safe so you can use modules and what not. So now I can use say Digest::SHA without tossing the baby out with the bath water (just using plperlu). Gaping security whole? Maybe, no more so than installing an insecure C/plperlu function as you have to edit postgresql.conf to change it. Right? Right. I'll emphasise the point that only plperlu code has access to anything loaded by plperl.on_init (aka .on_perl_init). Currently plperl code doesn't. There seemed to be some confusion upthread about how the GUCs work together so I'll recap here (using the original GUC names): When plperl.so is loaded (possibly in the postmaster due to shared_preload_libraries) a perl interpreter is created using whatever version of perl the server was configured with. That perl is initialized as if it had been started using: perl -e $(cat plc_perlboot.pl) If on_perl_init is set then the the initialization is effectively: perl -e $(cat plc_perlboot.pl) -e $on_perl_init That perl interpreter is now in a virgin 'unspecialized' state. From that state the interpreter may become either a plperl or plperlu interpreter depending on whichever language is first used. When an interpreter transitions from the initial state to become specialized for plperlu for a particular user then plperl.on_untrusted_init is eval'd. When an interpreter transitions from the initial state to become specialized for plperl then plc_safe_ok.pl is executed to create the Safe compartment and then plperl.on_trusted_init is eval'd *inside* the compartment. So, if all GUCs were set then plperl code would run in a perl initialized with on_perl_init + on_trusted_init, and plperlu code would run in a perl initialized with on_perl_init + on_untrusted_init. To add some context, I envisage plperl.on_perl_init being a stable value that typically pre-loads a set of modules etc., while the on_*trusted_init GUCs will typically be used for short-term debug/testing. Which is why on_untrusted_init (SUSET) is still useful with on_perl_init (SUSET). Maybe we should have: plperl.on_plperl_safe_init (runs inside of the safe, PGC_USERSET) plperl.on_plperl_init (runs outside safe, PGC_SUSET) plperl.on_plpleru_init (PGC_SUSET) Which, except
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 03, 2010 at 10:18:51AM -0700, Alex Hunsaker wrote: On Wed, Feb 3, 2010 at 09:31, Tim Bunce tim.bu...@pobox.com wrote: On Tue, Feb 02, 2010 at 08:42:21PM -0700, Alex Hunsaker wrote: On Sat, Jan 30, 2010 at 08:49, Tim Bunce tim.bu...@pobox.com wrote: SPI functions are not available when the code is run. Hrm, we might want to stick why in the docs or as a comment somewhere. I think this was the main concern? * We call a plperl function for the first time in a session, causing plperl.so to be loaded. This happens in the context of a superuser calling a non-superuser security definer function, or perhaps vice versa. Whose permissions apply to whatever the on_load code tries to do? (Hint: every answer is wrong.) It's hard to convey the underlying issues in a sentence or two. I tried. I'm not sure it's worth it. (Though if anyone thinks it is I'd be happy to get some specific suggestions for the wording to use.) All I know is I thought hrm... Why cant you have SPI ? It seems useful and I dont immediately see why its a bad idea. So I dug up the old threads. Im just afraid say in a year or two we will forget why we disallow it. Ah, yes, a comment is certainly easier to write up. I'll add one and include a link to the relevant thread in the archives. Thanks for the example. There does seem to be the risk that I may not have plperl GRANTed but I can make any plperl function elog(ERROR) as long as they have not loaded plperl via a plperl_safe_init. We can probably fix that if people think its a valid dos/attack vector. Interesting thought. If this is a valid concern (as it seems to be) then I could add some logic to check if the language has been GRANTed. (I.e. ignore on_plperl_init if the user can't write plperl code, and ignore on_plperlu_init if the user can't write plperlu code.) Well Im not sure. As a user I can probably cause havok just by passing interesting values to a function. It does seem inconsistent that you cant create plperl functions but you can potentially modify SHARED. In-fact if you have a security definer plperl function it seems quite scary that they could change values in SHARED. But any plperl function can do that anyway. (do we have/need documentation that SHARED in a plperl security definer function is unsafe?) So I dont think its *that* big of deal as long as the GRANT check is in place. I don't see a significant issue in security definer and %_SHARED from what you've said above. Authors of security definer functions should naturally take care in how they use their argument values and %_SHARED. I do see a need for a GRANT check and I'm adding one now (based on the code in CreateFunction() in functioncmds.c - thanks to RhodiumToad on IRC for the pointer). Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 03, 2010 at 02:04:47PM -0500, Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
%_SHARED has been around for several years now, and if there are genuine
security concerns about it ISTM they would apply today, regardless of
these patches.
Yes. I am not at all happy about inserting nonstandard permissions
checks into GUC assign hooks --- they are not really meant for that
and I think there could be unexpected consequences. Without a serious
demonstration of a real problem that didn't exist before, I'm not in
favor of it.
I wasn't thinking of using GUC assign hooks (as that simply hadn't
occured to me). I was thinking of just ignoring on_plperl_init if
the user wasn't allowed to use the plperl language. Something like:
if (user_is_su_or_has_usage_of('plperl')) {
... eval the on_plperl_init code ..
}
I think a more reasonable answer is just to add a documentation note
pointing out that %_SHARED should be considered insecure in a multi-user
database.
That's seems worth anyway. I'll add a note along those lines.
What I was actually wondering about, however, is the extent to which
the semantics of Perl code could be changed from an on_init hook ---
is there any equivalent of changing search_path or otherwise creating
trojan-horse code that might be executed unexpectedly?
This seems like a reasonable 'vector of first choice':
SET plperl.on_plperl_init = '$SIG{__WARN__} = sub { ... }';
and then do something to trigger a warning from some existing plperl
function. So I think the answer is yes.
And if so is there any point in trying to guard against it?
AIUI there isn't anything that can be done in on_init that couldn't be
done in somebody else's function anyhow.
(The issue here is with on_plperl_init, not on_init or on_plperlu_init as
they're SUSET).
There isn't anything that can be done in on_plperl_init that can't be
done in plperl in terms of what perl code can be compiled.
It seems there is a plausable vector for trojan-horse code though, so I
think the key issue if this could be exploited in a security definer
scenario.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Package namespace and Safe init cleanup for plperl UPDATE 3 [PATCH]
On Mon, Feb 01, 2010 at 07:53:05PM -0700, Alex Hunsaker wrote:
On Mon, Feb 1, 2010 at 03:58, Tim Bunce tim.bu...@pobox.com wrote:
On Sat, Jan 30, 2010 at 06:38:59PM -0700, Alex Hunsaker wrote:
plc_safe_ok.pl seems to loose its CVS $PostgreSQL$ keyword.
Probably a slip-up when I merged the changes from HEAD up through my
chain of branches.
Can you send an updated patch? I think Andrew will probably fix it up
anyway but better safe than sorry.
Attached. I'll add it to the commitfest.
Anyway yes I agree, but I thought I should at least raise it for
discussion. You'll notice the patch has been marked Ready for
Commiter this whole time. =)
Thanks.
Tim.
diff --git a/src/pl/plperl/expected/plperl.out b/src/pl/plperl/expected/plperl.out
index ebf9afd..0e7c65d 100644
*** a/src/pl/plperl/expected/plperl.out
--- b/src/pl/plperl/expected/plperl.out
*** CONTEXT: PL/Perl anonymous code block
*** 577,579
--- 577,584
DO $do$ use strict; my $name = foo; my $ref = $$name; $do$ LANGUAGE plperl;
ERROR: Can't use string (foo) as a SCALAR ref while strict refs in use at line 1.
CONTEXT: PL/Perl anonymous code block
+ -- check that we can use warnings (in this case to turn a warn into an error)
+ -- yields ERROR: Useless use of length in void context
+ DO $do$ use warnings FATAL = qw(void) ; length abc ; 1; $do$ LANGUAGE plperl;
+ ERROR: Useless use of length in void context at line 1.
+ CONTEXT: PL/Perl anonymous code block
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index 5d2e962..74b2a47 100644
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 1,26
# $PostgreSQL$
PostgreSQL::InServer::Util::bootstrap();
use strict;
use warnings;
use vars qw(%_SHARED);
! sub ::plperl_warn {
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
! elog(NOTICE, $msg);
}
! $SIG{__WARN__} = \::plperl_warn;
! sub ::plperl_die {
(my $msg = shift) =~ s/\(eval \d+\) //g;
die $msg;
}
! $SIG{__DIE__} = \::plperl_die;
! sub ::mkfuncsrc {
my ($name, $imports, $prolog, $src) = @_;
my $BEGIN = join \n, map {
--- 1,30
# $PostgreSQL$
+ use 5.008001;
+
PostgreSQL::InServer::Util::bootstrap();
+ package PostgreSQL::InServer;
+
use strict;
use warnings;
use vars qw(%_SHARED);
! sub plperl_warn {
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
! ::elog(::NOTICE, $msg);
}
! $SIG{__WARN__} = \plperl_warn;
! sub plperl_die {
(my $msg = shift) =~ s/\(eval \d+\) //g;
die $msg;
}
! $SIG{__DIE__} = \plperl_die;
! sub mkfuncsrc {
my ($name, $imports, $prolog, $src) = @_;
my $BEGIN = join \n, map {
*** sub ::mkfuncsrc {
*** 32,44
$name =~ s/\\//g;
$name =~ s/::|'/_/g; # avoid package delimiters
! return qq[ undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
}
# see also mksafefunc() in plc_safe_ok.pl
! sub ::mkunsafefunc {
no strict; # default to no strict for the eval
! my $ret = eval(::mkfuncsrc(@_));
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
--- 36,48
$name =~ s/\\//g;
$name =~ s/::|'/_/g; # avoid package delimiters
! return qq[ package main; undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
}
# see also mksafefunc() in plc_safe_ok.pl
! sub mkunsafefunc {
no strict; # default to no strict for the eval
! my $ret = eval(mkfuncsrc(@_));
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
*** sub ::encode_array_literal {
*** 67,73
sub ::encode_array_constructor {
my $arg = shift;
! return quote_nullable($arg)
if ref $arg ne 'ARRAY';
my $res = join , , map {
(ref $_) ? ::encode_array_constructor($_)
--- 71,77
sub ::encode_array_constructor {
my $arg = shift;
! return ::quote_nullable($arg)
if ref $arg ne 'ARRAY';
my $res = join , , map {
(ref $_) ? ::encode_array_constructor($_)
diff --git a/src/pl/plperl/plc_safe_ok.pl b/src/pl/plperl/plc_safe_ok.pl
index e3666f2..5d0fc93 100644
*** a/src/pl/plperl/plc_safe_ok.pl
--- b/src/pl/plperl/plc_safe_ok.pl
***
*** 1,43
-
# $PostgreSQL$
use strict;
! use vars qw($PLContainer);
- $PLContainer = new Safe('PLPerl');
$PLContainer-permit_only(':default');
$PLContainer-permit(qw[:base_math !:base_io sort time require]);
- $PLContainer-share(qw[elog return_next
- spi_query spi_fetchrow spi_cursor_close spi_exec_query
- spi_prepare spi_exec_prepared spi_query_prepared spi_freeplan
- DEBUG LOG INFO NOTICE WARNING ERROR %_SHARED
- quote_literal quote_nullable quote_ident
- encode_bytea decode_bytea
- encode_array_literal encode_array_constructor
- looks_like_number
- ]);
! # Load widely useful pragmas into the container to make them available.
! # (Temporarily enable caller here as work around for bug in perl 5.10,
! # which changed the way its Safe.pm works. It is quite
Re: [HACKERS] Package namespace and Safe init cleanup for plperl UPDATE 3 [PATCH]
On Sat, Jan 30, 2010 at 06:38:59PM -0700, Alex Hunsaker wrote:
On Sat, Jan 30, 2010 at 16:16, Tim Bunce tim.bu...@pobox.com wrote:
This is an update to the final plperl patch in the series from me.
Changes in the original patch:
plc_safe_ok.pl seems to loose its CVS $PostgreSQL$ keyword.
Probably a slip-up when I merged the changes from HEAD up through my
chain of branches.
- Ensure Safe container opmask is restored even if @EvalInSafe code
throws an exception.
Maybe we could be a bit smarter about this and avoid the problem?
Im thinking either (for @ShareIntoSafe as well):
1) always reinit @EvalInSafe at the top of plc_safe_ok.pl
our @EvalInSafe = [ ];
Seems like a non-starter, why have the 'our' at all?
Yeap.
2)Change EvalInSafe to be a hash so at least the problem with
duplicates goes away:
$EvalInSafe{'strict'} = 'caller';
Then again maybe its fine the way it is. Thoughts?
A better approach would be for the plperl.c code to keep track of
initialization with a finer granularity. Specifically track the fact
that plc_safe_ok.pl ran ok so not re-run it if on_trusted_init fails.
But that would be a more invasive change for no significant gain so
didn't seem appropriate at this point.
The current code works fine, and behaves well in failure modes, so I
think it's okay the way it is.
I hope to work on plperl.c some more for the 9.1 release (if my
employer's generosity continues). Mainly to switch to using
PERL_NO_GET_CONTEXT to simplify state management and improve
performance (getting rid of the many many hidden calls to
pthread_getspecific). That would be a good time to rework this area.
This makes me think maybe we should not expose it at all. Its not
like you can tell people oh you have something you need in your plperl
safe? Just stick it in @PostgreSQL::InServer::safe::EvalInSafe. As
we might change this *undocumented* interface in the future.
That being said *im* ok with it. Its useful from a debug standpoint.
Yes. And, as I mentioned previously, I expect people like myself, David
Wheeler, and others to experiment with the undocumented functionality
and define and document a good API to it for the 9.1 release.
I'd much rather get this change in than shoot for a larger change that
doesn't get committed due to long-running discussions. (Which seems
more likely as Andrew's going to be less available for the rest of the
commitfest.)
Tim.
p.s. If there is interest in defining a documented API (for DBAs to
control what gets loaded into Safe and shared with it) for *9.0*
then that could be worked on, once this pach is in, ready for the
next commitfest.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Package namespace and Safe init cleanup for plperl UPDATE 3 [PATCH]
On Mon, Feb 01, 2010 at 10:46:10AM -0500, Robert Haas wrote: On Mon, Feb 1, 2010 at 5:58 AM, Tim Bunce tim.bu...@pobox.com wrote: p.s. If there is interest in defining a documented API (for DBAs to control what gets loaded into Safe and shared with it) for *9.0* then that could be worked on, once this pach is in, ready for the next commitfest. This is the last CommitFest for 9.0. It's time to wind down development on this release and work on trying to get the release stabilized and out the door. This isn't intended as a disparagement of the work you're doing; I've thought about using PL/perl in the past and decided against it exactly because of some of the issues you're now fixing. But we're really out of time to get things done for 9.0. Understood Robert. No problem. (You can't blame me for trying ;-) Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Add on_perl_init and proper destruction to plperl UPDATE v3 [PATCH]
On Fri, Jan 29, 2010 at 09:10:48PM -0500, Andrew Dunstan wrote: Tim Bunce wrote: This is an updated version of the third of the patches to be split out from the former 'plperl feature patch 1'. It includes changes following discussions with Tom Lane and others. Changes in this patch: - Added plperl.on_perl_init GUC for DBA use (PGC_SIGHUP) SPI functions are not available when the code is run. - Added interpreter destruction behaviour Hooked via on_proc_exit(). Only has any effect for normal shutdown. END blocks, if any, are run. SPI functions will die if called at this time. This updated version no longer tries to call object destructors. I've added a note in the Limitations section of the PL/Perl docs. It also adds a PERL_SET_CONTEXT() that's needed but was missing. I have committed this. Many thanks. Tim, can you rebase the last two patches against current CVS HEAD? ASAP... Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]
On Fri, Jan 29, 2010 at 08:07:30PM -0700, Alex Hunsaker wrote:
A couple of comments. *note* I have not tested this as a whole yet
(due to rejects).
in plc_perboot.pl
+$funcsrc .= qq[ package main; undef *{'$name'}; *{'$name'} = sub {
$BEGIN $prolog $src } ];
Any thoughts on using a package other than main? Maybe something like
package PlPerl or what not? Not that I think it really matters...
But it might be nicer to see in say NYTprof ?
The only interface Safe provides for sharing things with the compartment
shares them with the root package of the compartment which, from within
the compartment, is 'main::'.
It's an unfortunate limitation of Safe. I could have added more code to
wordaround that but opted to keep it simple for this commitfest.
in plc_safe_ok.pl
+use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);
Is there some reason not to use my here? The only reason I can think
of is you want the *_init gucs to be able to stick things into
@ShareIntoSafe and friends? And if so should we document that
somewhere (or was that in an earlier patch that i missed?)?
It's a step towards providing an interface to give the DBA control over
what gets loaded into the Safe compartment and what gets shared with it.
I opted to not try to define a formal documented interface because I
felt it was likely to be a source of controversy and/or bikeshedding.
This late in the game I didn't want to take that chance.
I'd rather a few brave souls get some experience with it as-as, and collect
some good use-cases, before proposing a formal documented API.
Also whats the use case for $SafeClass? Multiple versions of Safe.pm?
Other modules that are like Safe.pm? Im just curious...
It's possible someone might want to use an alternative module.
Most likely their own local subclass of Safe that adds extra features.
I'd explored that when trying to integrate NYTProf. It seemed worth
at least making possible.
Hrm I also dont see the point of the new use Safe; as we still eval
it in in plperl_safe_init() care to enlighten a lost soul?
It just makes the file more self-contained for syntax checking:
perl -cw plc_safeboot.pl
Other than those really quite minor questions that are arguably me
nitpicking... It looks great to me.
Great, thanks Alex!
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
This is an update the fourth of the patches to be split out from the
former 'plperl feature patch 1'.
Changes in this patch:
- Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs
on_trusted_init is PGC_USERSET, on_untrusted_init is PGC_SUSET
SPI functions are not available when the code is run.
Errors are detected and reported as ereport(ERROR, ...)
Corresponding documentation.
- select_perl_context() state management improved
An error during interpreter initialization will leave
the state (interp_state etc) unchanged.
- The utf8fix code has been greatly simplified.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index ea56b99..0add7d1 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** CREATE TRIGGER test_valid_id_trig
*** 1058,1066
or subtransaction to be aborted.
/para
para
! The perl code is limited to a single string. Longer code can be placed
! into a module and loaded by the literalon_perl_init/ string.
! Examples:
programlisting
plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;'
--- 1058,1066
or subtransaction to be aborted.
/para
para
!The perl code is limited to a single string. Longer code can be placed
!into a module and loaded by the literalon_perl_init/ string.
!Examples:
programlisting
plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;'
*** plplerl.on_perl_init = 'use lib /my/app
*** 1077,1082
--- 1077,1128
/listitem
/varlistentry
+ varlistentry id=guc-plperl-on-trusted-init xreflabel=plperl.on_trusted_init
+ termvarnameplperl.on_trusted_init/varname (typestring/type)/term
+ indexterm
+primaryvarnameplperl.on_trusted_init/ configuration parameter/primary
+ /indexterm
+ listitem
+para
+Specifies perl code to be executed when the literalplperl/ language
+is first used in a session. Changes made after the literalplperl/
+language has been used will have no effect.
+The perl code can only perform trusted operations.
+The SPI functions are not available when this code is executed.
+/para
+para
+ If the code fails with an error it will abort the initialization and
+ propagate out to the calling query, causing the current transaction or
+ subtransaction to be aborted. Any changes within the perl won't be
+ undone. If the literalplperl/ language is used again the
+ initialization will be repeated.
+/para
+ /listitem
+ /varlistentry
+
+ varlistentry id=guc-plperl-on-untrusted-init xreflabel=plperl.on_untrusted_init
+ termvarnameplperl.on_untrusted_init/varname (typestring/type)/term
+ indexterm
+primaryvarnameplperl.on_untrusted_init/ configuration parameter/primary
+ /indexterm
+ listitem
+para
+Specifies perl code to be executed when the literalplperlu/ perl language
+ is first used in a session. Changes made after the literalplperlu/
+ language has been used will have no effect.
+The SPI functions are not available when this code is executed.
+Only superusers can change this settings.
+/para
+para
+ If the code fails with an error it will abort the initialization and
+ propagate out to the calling query, causing the current transaction or
+ subtransaction to be aborted. Any changes within the perl won't be
+ undone. If the literalplperlu/ language is used again the
+ initialization will be repeated.
+/para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict
termvarnameplperl.use_strict/varname (typeboolean/type)/term
indexterm
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a9bb003..165e90d 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*** PERLCHUNKS = plc_perlboot.pl plc_safe_ba
*** 41,47
SHLIB_LINK = $(perl_embed_ldflags)
REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperlu
# if Perl can support two interpreters in one backend,
# test plperl-and-plperlu cases
ifneq ($(PERL),)
--- 41,47
SHLIB_LINK = $(perl_embed_ldflags)
REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperl_init plperlu
# if Perl can support two interpreters in one backend,
# test plperl-and-plperlu cases
ifneq ($(PERL),)
diff --git
Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]
On Sat, Jan 30, 2010 at 11:08:26AM -0700, Alex Hunsaker wrote:
On Sat, Jan 30, 2010 at 07:51, Tim Bunce tim.bu...@pobox.com wrote:
On Fri, Jan 29, 2010 at 08:07:30PM -0700, Alex Hunsaker wrote:
Other than those really quite minor questions that are arguably me
nitpicking... It looks great to me.
Great, thanks Alex!
I marked it as ready, though ill add a comment that it depends on the
other patch to apply cleanly (and if that one gets rebased...
obviously this one probably will need to as well).
I've rebased and reposted the other patch (Add on_trusted_init and
on_untrusted_init to plperl) and added it to the commitfest.
I've rebasing this one now and making one minor change: I'm adding an
if (not our $_init++) {
...
}
around the code that adds items to @EvalInSafe and @ShareIntoSafe to
avoid any problems with on_trusted_init code that causes exceptions.
Currently an exception from on_trusted_init will leave the plperl
interpreter in the 'not yet setup' state. So the next use runs
the plc_safe_ok code and the on_trusted_init code again.
This change makes the plc_safe_ok code idempotent so the re-execution
won't cause any problems (except for harmless warnings about the
safe_eval and mksafefunc subs being redefined).
Patch to follow (after getting the kids to bed).
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Package namespace and Safe init cleanup for plperl UPDATE 3 [PATCH]
This is an update to the final plperl patch in the series from me.
Changes in the original patch:
- Moved internal functions out of main:: namespace
into PostgreSQL::InServer and PostgreSQL::InServer::safe
- Restructured Safe compartment setup code
to generalize and separate the data from the logic.
Neither change has any user visible effects.
Additional changes in the second version:
- Further generalized the 'what to load into Safe compartment' logic.
- Added the 'warnings' pragma to the list of modules to load into Safe.
So plperl functions can now use warnings; - added test for that.
- Added 'use 5.008001;' to plc_perlboot.pl as a run-time check to
complement the configure-time check added by Tom Lane recently.
Additional changes in this version:
- Rebased over recent HEAD plus on_trusted_init patch
- Made plc_safe_ok.pl code idempotent to avoid risk of problems
from repeated initialization attempts e.g. if on_trusted_init code
throws an exception so initialization doesn't complete.
- Fixed 'require strict' to enable 'caller' opcode
(needed for Perl =5.10)
- Ensure Safe container opmask is restored even if @EvalInSafe code
throws an exception.
- Changed errmsg(didn't get a GLOB ...) to use errmsg_internal().
Tim.
diff --git a/src/pl/plperl/expected/plperl.out b/src/pl/plperl/expected/plperl.out
index ebf9afd..0e7c65d 100644
*** a/src/pl/plperl/expected/plperl.out
--- b/src/pl/plperl/expected/plperl.out
*** CONTEXT: PL/Perl anonymous code block
*** 577,579
--- 577,584
DO $do$ use strict; my $name = foo; my $ref = $$name; $do$ LANGUAGE plperl;
ERROR: Can't use string (foo) as a SCALAR ref while strict refs in use at line 1.
CONTEXT: PL/Perl anonymous code block
+ -- check that we can use warnings (in this case to turn a warn into an error)
+ -- yields ERROR: Useless use of length in void context
+ DO $do$ use warnings FATAL = qw(void) ; length abc ; 1; $do$ LANGUAGE plperl;
+ ERROR: Useless use of length in void context at line 1.
+ CONTEXT: PL/Perl anonymous code block
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index 5d2e962..74b2a47 100644
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 1,26
# $PostgreSQL$
PostgreSQL::InServer::Util::bootstrap();
use strict;
use warnings;
use vars qw(%_SHARED);
! sub ::plperl_warn {
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
! elog(NOTICE, $msg);
}
! $SIG{__WARN__} = \::plperl_warn;
! sub ::plperl_die {
(my $msg = shift) =~ s/\(eval \d+\) //g;
die $msg;
}
! $SIG{__DIE__} = \::plperl_die;
! sub ::mkfuncsrc {
my ($name, $imports, $prolog, $src) = @_;
my $BEGIN = join \n, map {
--- 1,30
# $PostgreSQL$
+ use 5.008001;
+
PostgreSQL::InServer::Util::bootstrap();
+ package PostgreSQL::InServer;
+
use strict;
use warnings;
use vars qw(%_SHARED);
! sub plperl_warn {
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
! ::elog(::NOTICE, $msg);
}
! $SIG{__WARN__} = \plperl_warn;
! sub plperl_die {
(my $msg = shift) =~ s/\(eval \d+\) //g;
die $msg;
}
! $SIG{__DIE__} = \plperl_die;
! sub mkfuncsrc {
my ($name, $imports, $prolog, $src) = @_;
my $BEGIN = join \n, map {
*** sub ::mkfuncsrc {
*** 32,44
$name =~ s/\\//g;
$name =~ s/::|'/_/g; # avoid package delimiters
! return qq[ undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
}
# see also mksafefunc() in plc_safe_ok.pl
! sub ::mkunsafefunc {
no strict; # default to no strict for the eval
! my $ret = eval(::mkfuncsrc(@_));
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
--- 36,48
$name =~ s/\\//g;
$name =~ s/::|'/_/g; # avoid package delimiters
! return qq[ package main; undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
}
# see also mksafefunc() in plc_safe_ok.pl
! sub mkunsafefunc {
no strict; # default to no strict for the eval
! my $ret = eval(mkfuncsrc(@_));
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
*** sub ::encode_array_literal {
*** 67,73
sub ::encode_array_constructor {
my $arg = shift;
! return quote_nullable($arg)
if ref $arg ne 'ARRAY';
my $res = join , , map {
(ref $_) ? ::encode_array_constructor($_)
--- 71,77
sub ::encode_array_constructor {
my $arg = shift;
! return ::quote_nullable($arg)
if ref $arg ne 'ARRAY';
my $res = join , , map {
(ref $_) ? ::encode_array_constructor($_)
diff --git a/src/pl/plperl/plc_safe_ok.pl b/src/pl/plperl/plc_safe_ok.pl
index e3666f2..b87284c 100644
*** a/src/pl/plperl/plc_safe_ok.pl
--- b/src/pl/plperl/plc_safe_ok.pl
***
*** 1,43
! # $PostgreSQL$
! use strict;
! use vars qw($PLContainer);
- $PLContainer = new Safe('PLPerl');
$PLContainer-permit_only(':default');
$PLContainer-permit(qw[:base_math !:base_io
Re: [HACKERS] Add on_perl_init and proper destruction to plperl UPDATED [PATCH]
On Thu, Jan 28, 2010 at 11:02:23PM -0500, Andrew Dunstan wrote: Tim Bunce wrote: This is an updated version of the third of the patches to be split out from the former 'plperl feature patch 1'. It includes changes following discussions with Tom Lane and others. Changes in this patch: - Added plperl.on_perl_init GUC for DBA use (PGC_SIGHUP) SPI functions are not available when the code is run. - Added interpreter destruction behaviour Hooked via on_proc_exit(). Only has any effect for normal shutdown. END blocks, if any, are run then objects are destroyed, calling their DESTROY methods, if any. SPI functions will die if called at this time. This patch is giving me a build error on Windows: undefined reference to `Perl_sv_clean_objs' Ah, phooey. That's technically a private function so isn't exported on platforms that support selective exporting. The options are either to go back to calling perl_destruct(), which would then require careful auditing of what perl_destruct actually does, or do simply not bother destroying objects. I'm going to go for the latter. Time is short and calling END blocks is still a major step forward. (Anyone who needs objects destroyed can probably arrange that themselves via an END block.) Updated patch to follow... Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plperl compiler warning
On Thu, Jan 28, 2010 at 07:49:37PM +, Tim Bunce wrote: I think I missed this because the Xcode compiler on Snow Leopard is fairly old (gcc 4.2.1). For the record, gcc 4.2.1 does report the error. I'd missed it because I'd done most of my builds with perl 5.8.x and the notnull attributes was added later, in 5.10.0. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Add on_perl_init and proper destruction to plperl UPDATE v3 [PATCH]
This is an updated version of the third of the patches to be
split out from the former 'plperl feature patch 1'.
It includes changes following discussions with Tom Lane and others.
Changes in this patch:
- Added plperl.on_perl_init GUC for DBA use (PGC_SIGHUP)
SPI functions are not available when the code is run.
- Added interpreter destruction behaviour
Hooked via on_proc_exit().
Only has any effect for normal shutdown.
END blocks, if any, are run.
SPI functions will die if called at this time.
This updated version no longer tries to call object destructors.
I've added a note in the Limitations section of the PL/Perl docs.
It also adds a PERL_SET_CONTEXT() that's needed but was missing.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 5fa7e3a..c4d5d8e 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** CREATE TRIGGER test_valid_id_trig
*** 1028,1034
/para
/sect1
! sect1 id=plperl-missing
titleLimitations and Missing Features/title
para
--- 1028,1098
/para
/sect1
! sect1 id=plperl-under-the-hood
! titlePL/Perl Under the Hood/title
!
! sect2 id=plperl-config
! titleConfiguration/title
!
! para
! This section lists configuration parameters that affect applicationPL/Perl/.
! To set any of these parameters before applicationPL/Perl/ has been loaded,
! it is necessary to have added quoteliteralplperl// to the
! xref linkend=guc-custom-variable-classes list in
! filenamepostgresql.conf/filename.
! /para
!
! variablelist
!
! varlistentry id=guc-plperl-on-perl-init xreflabel=plperl.on_perl_init
! termvarnameplperl.on_perl_init/varname (typestring/type)/term
! indexterm
!primaryvarnameplperl.on_perl_init/ configuration parameter/primary
! /indexterm
! listitem
!para
!Specifies perl code to be executed when a perl interpreter is first initialized.
!The SPI functions are not available when this code is executed.
!If the code fails with an error it will abort the initialization of the interpreter
!and propagate out to the calling query, causing the current transaction
!or subtransaction to be aborted.
!/para
!para
! The perl code is limited to a single string. Longer code can be placed
! into a module and loaded by the literalon_perl_init/ string.
! Examples:
! programlisting
! plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;'
! /programlisting
!/para
!para
!Initialization will happen in the postmaster if the plperl library is included
!in literalshared_preload_libraries/ (see xref linkend=shared_preload_libraries),
!in which case extra consideration should be given to the risk of destabilizing the postmaster.
!/para
!para
!This parameter can only be set in the postgresql.conf file or on the server command line.
!/para
! /listitem
! /varlistentry
!
! varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict
! termvarnameplperl.use_strict/varname (typeboolean/type)/term
! indexterm
!primaryvarnameplperl.use_strict/ configuration parameter/primary
! /indexterm
! listitem
!para
!When set true subsequent compilations of PL/Perl functions have the literalstrict/ pragma enabled.
!This parameter does not affect functions already compiled in the current session.
!/para
! /listitem
! /varlistentry
!
! /variablelist
!
! sect2 id=plperl-missing
titleLimitations and Missing Features/title
para
*** CREATE TRIGGER test_valid_id_trig
*** 1063,1072
--- 1127,1146
literalreturn_next/literal for each row returned, as shown
previously.
/para
+ /listitem
+ listitem
+ para
+ When a session ends normally, not due to a fatal error, any
+ literalEND/ blocks that have been defined are executed.
+ Currently no other actions are performed. Specifically, file handles are
+ not flushed and objects are not destroyed.
+ /para
/listitem
/itemizedlist
/para
+ /sect2
+
/sect1
/chapter
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index 24e2487..5d2e962 100644
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 2,8
# $PostgreSQL$
PostgreSQL::InServer::Util::bootstrap();
- PostgreSQL::InServer::SPI::bootstrap();
use strict;
use warnings;
--- 2,7
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 9277072..8b1d697 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***
*** 27,32
--- 27,33
#include miscadmin.h
#include nodes/makefuncs.h
#include parser/parse_type.h
+
Re: [HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 06:33:19PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Wed, Jan 27, 2010 at 11:28:02AM -0500, Tom Lane wrote: Really? We've found that gprof, for instance, doesn't exactly have zero interaction with the rest of the backend --- there's actually a couple of different bits in there to help it along, including a behavioral change during shutdown. I rather doubt that Perl profilers would turn out much different. Devel::NYTProf (http://blog.timbunce.org/tag/nytprof/) has zero interaction with the rest of the backend. I don't have to read any further than the place where it says doesn't work if you call both plperl and plperlu to realize that that's quite false. NYTProf is not, currently, multiplicity-safe. That's a limitation I intend to fix. Maybe we have different definitions of what a software interaction is... Doing _anything_ in the backend is an interaction of some kind, e.g., shifting later memory allocations to a different address. But that's not a very practical basis for a definition. From what you said, quoted above, it seemed that your definition of interaction with the rest of the backend was more much more direct. The specific example you gave related to the backend code needing to be modified to support the gprof profiler. Clearly that's not the case for NYTProf. We're splitting hairs now. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 06:27:50PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: Okay. I could change the callback code to ignore calls if proc_exit_inprogress is false. So an abnormal shutdown via exit() wouldn't involve plperl at all. (Alternatively I could use use on_proc_exit() instead of atexit() to register the callback.) Use on_proc_exit please. I will continue to object to any attempt to hang arbitrary processing on atexit(). Ok. An advantage of on_proc_exit from your end is that it should allow you to not have to try to prevent the END blocks from using SPI, as that would still be perfectly functional when your callback gets called. (Starting a new transaction would be a good idea though, cf Async_UnlistenOnExit.) I'm surprised that you're suggesting that END block should be allowed to interact with the backend via SPI. It seems to go against what you've said previously about code running at shutdown. I've no use-case for that so I'm happy to leave it disabled. If someone does have a sane use-case, please let me know. It can always be enabled later. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plperl compiler warning
On Thu, Jan 28, 2010 at 06:31:19AM -0800, Joe Conway wrote: Last night I noted the following warning: plperl.c: In function ‘plperl_create_sub’: plperl.c:1117: warning: null argument where non-null required (argument 2) The master branch of my git clone says line 1117 is: subref = newRV_inc((SV*)GvCVu((GV*)sub_glob)); Does that match yours? (If not, what is the text on the line?) What perl version are you using? What compiler version are you using? Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Add on_perl_init and proper destruction to plperl UPDATED [PATCH]
This is an updated version of the third of the patches to be split out
from the former 'plperl feature patch 1'.
It includes changes following discussions with Tom Lane and others.
Changes in this patch:
- Added plperl.on_perl_init GUC for DBA use (PGC_SIGHUP)
SPI functions are not available when the code is run.
- Added interpreter destruction behaviour
Hooked via on_proc_exit().
Only has any effect for normal shutdown.
END blocks, if any, are run then objects are
destroyed, calling their DESTROY methods, if any.
SPI functions will die if called at this time.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 5fa7e3a..06c63df 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** CREATE TRIGGER test_valid_id_trig
*** 1028,1034
/para
/sect1
! sect1 id=plperl-missing
titleLimitations and Missing Features/title
para
--- 1028,1098
/para
/sect1
! sect1 id=plperl-under-the-hood
! titlePL/Perl Under the Hood/title
!
! sect2 id=plperl-config
! titleConfiguration/title
!
! para
! This section lists configuration parameters that affect applicationPL/Perl/.
! To set any of these parameters before applicationPL/Perl/ has been loaded,
! it is necessary to have added quoteliteralplperl// to the
! xref linkend=guc-custom-variable-classes list in
! filenamepostgresql.conf/filename.
! /para
!
! variablelist
!
! varlistentry id=guc-plperl-on-perl-init xreflabel=plperl.on_perl_init
! termvarnameplperl.on_perl_init/varname (typestring/type)/term
! indexterm
!primaryvarnameplperl.on_perl_init/ configuration parameter/primary
! /indexterm
! listitem
!para
!Specifies perl code to be executed when a perl interpreter is first initialized.
!The SPI functions are not available when this code is executed.
!If the code fails with an error it will abort the initialization of the interpreter
!and propagate out to the calling query, causing the current transaction
!or subtransaction to be aborted.
!/para
!para
! The perl code is limited to a single string. Longer code can be placed
! into a module and loaded by the literalon_perl_init/ string.
! Examples:
! programlisting
! plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;'
! /programlisting
!/para
!para
!Initialization will happen in the postmaster if the plperl library is included
!in literalshared_preload_libraries/ (see xref linkend=shared_preload_libraries),
!in which case extra consideration should be given to the risk of destabilizing the postmaster.
!/para
!para
!This parameter can only be set in the postgresql.conf file or on the server command line.
!/para
! /listitem
! /varlistentry
!
! varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict
! termvarnameplperl.use_strict/varname (typeboolean/type)/term
! indexterm
!primaryvarnameplperl.use_strict/ configuration parameter/primary
! /indexterm
! listitem
!para
!When set true subsequent compilations of PL/Perl functions have the literalstrict/ pragma enabled.
!This parameter does not affect functions already compiled in the current session.
!/para
! /listitem
! /varlistentry
!
! /variablelist
!
! sect2 id=plperl-missing
titleLimitations and Missing Features/title
para
*** CREATE TRIGGER test_valid_id_trig
*** 1067,1072
--- 1131,1138
/listitem
/itemizedlist
/para
+ /sect2
+
/sect1
/chapter
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index 24e2487..5d2e962 100644
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 2,8
# $PostgreSQL$
PostgreSQL::InServer::Util::bootstrap();
- PostgreSQL::InServer::SPI::bootstrap();
use strict;
use warnings;
--- 2,7
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 9277072..2202b0f 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***
*** 27,32
--- 27,33
#include miscadmin.h
#include nodes/makefuncs.h
#include parser/parse_type.h
+ #include storage/ipc.h
#include utils/builtins.h
#include utils/fmgroids.h
#include utils/guc.h
*** static HTAB *plperl_proc_hash = NULL;
*** 138,143
--- 139,146
static HTAB *plperl_query_hash = NULL;
static bool plperl_use_strict = false;
+ static char *plperl_on_perl_init = NULL;
+ static bool plperl_ending = false;
/* this is saved and restored by plperl_call_handler */
static plperl_call_data *current_call_data = NULL;
*** Datum plperl_validator(PG_FUNCTION_ARGS
*** 151,156
Re: [HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
On Thu, Jan 28, 2010 at 10:39:33AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Wed, Jan 27, 2010 at 06:27:50PM -0500, Tom Lane wrote: An advantage of on_proc_exit from your end is that it should allow you to not have to try to prevent the END blocks from using SPI, as that would still be perfectly functional when your callback gets called. (Starting a new transaction would be a good idea though, cf Async_UnlistenOnExit.) I'm surprised that you're suggesting that END block should be allowed to interact with the backend via SPI. It seems to go against what you've said previously about code running at shutdown. I think you have completely misunderstood what I'm complaining about. What I'm not happy about is executing operations at a point where they're likely to be ill-defined because the code is in the wrong state. In an early on_proc_exit hook, the system is for all practical purposes still fully functional, and so I don't see a reason for an arbitrary restriction on what the END blocks should be able to do. Ah, okay. I guess I missed your underlying concerns in: http://archives.postgresql.org/message-id/26766.1263149...@sss.pgh.pa.us For the record, [...] and I think it's a worse idea to run arbitrary user-defined code at backend shutdown (the END-blocks bit). (Or, to repeat myself in a different way: the no-SPI restriction is utterly useless to guard against my real concerns anyway. I see no point in it either here or elsewhere.) I've left it in the updated patch I've just posted. There are two more plperl patches in the current commitfest that I'd like to chaperone through to commit (in some form or other) first. Thanks for your help Tom. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Now the dust is settling on the on_perl_init patch I'd like to ask for clarification on this next patch. On Fri, Jan 15, 2010 at 12:35:06AM +, Tim Bunce wrote: This is the fourth of the patches to be split out from the former 'plperl feature patch 1'. Changes in this patch: I think the only controversial change is this one: - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs Both are PGC_USERSET. SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) + plperl.on_trusted_init runs inside the Safe compartment. As I recall, Tom had concerns over the combination of PGC_USERSET and before-first-use semantics. Would changing plperl.on_trusted_init and plperl.on_untrusted_init to PGC_BACKEND, so the user can't change the value after the session has started, resolve those concerns? Any other concerns with this patch? Tim. - select_perl_context() state management improved An error during interpreter initialization will leave the state (interp_state etc) unchanged. - The utf8fix code has been greatly simplified. Tim. diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml index 0054f5a..f2c91a9 100644 *** a/doc/src/sgml/plperl.sgml --- b/doc/src/sgml/plperl.sgml *** plplerl.on_perl_init = 'use lib /my/app *** 1079,1084 --- 1079,1120 /listitem /varlistentry + varlistentry id=guc-plperl-on-trusted-init xreflabel=plperl.on_trusted_init + termvarnameplperl.on_trusted_init/varname (typestring/type)/term + indexterm +primaryvarnameplperl.on_trusted_init/ configuration parameter/primary + /indexterm + listitem +para +Specifies perl code to be executed when the literalplperl/ perl interpreter +is first initialized in a session. The perl code can only perform trusted operations. +The SPI functions are not available when this code is executed. +Changes made after a literalplperl/ perl interpreter has been initialized will have no effect. +If the code fails with an error it will abort the initialization of the interpreter +and propagate out to the calling query, causing the current transaction +or subtransaction to be aborted. +/para + /listitem + /varlistentry + + varlistentry id=guc-plperl-on-untrusted-init xreflabel=plperl.on_untrusted_init + termvarnameplperl.on_untrusted_init/varname (typestring/type)/term + indexterm +primaryvarnameplperl.on_untrusted_init/ configuration parameter/primary + /indexterm + listitem +para +Specifies perl code to be executed when the literalplperlu/ perl interpreter +is first initialized in a session. +The SPI functions are not available when this code is executed. +Changes made after a literalplperlu/ perl interpreter has been initialized will have no effect. +If the code fails with an error it will abort the initialization of the interpreter +and propagate out to the calling query, causing the current transaction +or subtransaction to be aborted. +/para + /listitem + /varlistentry + varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict termvarnameplperl.use_strict/varname (typeboolean/type)/term indexterm diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index 7cd5721..f3cabad 100644 *** a/src/pl/plperl/GNUmakefile --- b/src/pl/plperl/GNUmakefile *** PERLCHUNKS = plc_perlboot.pl plc_safe_ba *** 41,47 SHLIB_LINK = $(perl_embed_ldflags) REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu ! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperlu # if Perl can support two interpreters in one backend, # test plperl-and-plperlu cases ifneq ($(PERL),) --- 41,47 SHLIB_LINK = $(perl_embed_ldflags) REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu ! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperl_init plperlu # if Perl can support two interpreters in one backend, # test plperl-and-plperlu cases ifneq ($(PERL),) diff --git a/src/pl/plperl/expected/plperl_init.out b/src/pl/plperl/expected/plperl_init.out index ...e69de29 . diff --git a/src/pl/plperl/expected/plperl_shared.out b/src/pl/plperl/expected/plperl_shared.out index 72ae1ba..c1c12c1 100644 *** a/src/pl/plperl/expected/plperl_shared.out --- b/src/pl/plperl/expected/plperl_shared.out *** *** 1,3 --- 1,7 + -- test plperl.on_plperl_init via the shared hash + -- (must be done before plperl is initialized) + -- testing on_trusted_init gets run, and that it can alter %_SHARED + SET
Re: [HACKERS] plperl compiler warning
On Thu, Jan 28, 2010 at 12:49:20PM -0500, Tom Lane wrote:
Joe Conway m...@joeconway.com writes:
I pull directly from CVS, not git, but in any case my line 1117 is
subref = newRV_inc((SV*)GvCVu((GV*)sub_glob));
so it appears to be the same
What perl version are you using?
What compiler version are you using?
I'm on stock Fedora 12:
I see the same on Fedora 11. The -E expansion of the line in question is
subref = Perl_newRV(((PerlInterpreter
*)pthread_getspecific((*Perl_Gthr_key_ptr(((void *)0),
(SV*)GV*)sub_glob)-sv_u.svu_gp)-gp_cvgen ? ((void *)0) :
(((GV*)sub_glob)-sv_u.svu_gp)-gp_cv));
so it's evidently unhappy about the fact that GvCVu can return null,
while Perl_newRV is declared __attribute__((nonnull(2))).
It looks to me like this is probably a live bug not just compiler
hypersensitivity.
Yes. (ISTR there have been cases where the notnull attribute was
misapplied to some perl functions, but that's not the case here.)
I think I missed this because the Xcode compiler on Snow Leopard is
fairly old (gcc 4.2.1).
Patch attached.
Tim.
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 9277072..2dd3458 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*** plperl_create_sub(plperl_proc_desc *prod
*** 1113,1120
if (count == 1) {
GV *sub_glob = (GV*)POPs;
! if (sub_glob SvTYPE(sub_glob) == SVt_PVGV)
! subref = newRV_inc((SV*)GvCVu((GV*)sub_glob));
}
PUTBACK;
--- 1113,1123
if (count == 1) {
GV *sub_glob = (GV*)POPs;
! if (sub_glob SvTYPE(sub_glob) == SVt_PVGV) {
! SV *sv = (SV*)GvCVu((GV*)sub_glob);
! if (sv)
! subref = newRV_inc(sv);
! }
}
PUTBACK;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
On Thu, Jan 28, 2010 at 12:12:58PM -0500, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. ITYM on_untrusted_init. Right, sorry, got 'em backwards. I've done that several times. The naming is tricky because it's very dependent on your point of view. The 'trusted' language is for running 'untrusted' code and the 'untrusted' language is for running 'trusted' code. The naming convention is unfortunate. Just an observation from a newbie. I imagine it's been pointed out before. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
On Thu, Jan 28, 2010 at 11:54:08AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: I think the only controversial change is this one: - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs Both are PGC_USERSET. SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) + plperl.on_trusted_init runs inside the Safe compartment. Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. Yes. Good point (though ITYM on_UNtrusted_init). I'll change that. As I recall, Tom had concerns over the combination of PGC_USERSET and before-first-use semantics. IIRC, what I was unhappy about was exposing shared library load as a time when interesting-to-the-user things would happen. It looks like you have got rid of that and instead made it happen at the first call of a plperl or plperlu function (respectively). That seems like a fairly reasonable way to work, and if it's defined that way, there doesn't seem any reason not to allow them to be USERSET/SUSET. Great. But it ought to be documented like that, not with vague phrases like when the interpreter is initialized --- that means nothing to users. I'll change that. One issue that ought to be mentioned is what about transaction rollback. One could argue that if the first plperl call is inside a transaction that later rolls back, then all the side effects of that should be undone. But we have no way to undo whatever happened inside perl, and at least in most likely uses of on_init there wouldn't be any advantage in trying to force a redo anyway. I think it's okay to just define it as happening once regardless of any transaction failure (but of course this had better be documented). Will do. Once the previous patch lands I'll post an update to this patch with those changes applied. Thanks. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 01:14:16AM -0500, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Tim Bunce wrote: - Added plperl.on_perl_init GUC for DBA use (PGC_SIGHUP) SPI functions are not available when the code is run. - Added normal interpreter destruction behaviour END blocks, if any, are run then objects are destroyed, calling their DESTROY methods, if any. SPI functions will die if called at this time. So, are there still objections to applying this patch? Yes. To focus the discussion I've looked back through all the messages from you that relate to this issue so I can summarize and try to address your objections. Some I've split or presented out of order, most relate to earlier (less restricted) versions of the patch before it was split out, and naturally they are lacking some context, so I've included archive URLs. Please forgive and correct me if I misrepresent you or your intent here. Regarding the utility of plperl.on_perl_init and END: http://archives.postgresql.org/message-id/18338.1260033...@sss.pgh.pa.us The question is not about whether we think it's useful; the question is about whether it's safe. I agree. Regarding visibility of changes to plperl.on_perl_init: http://archives.postgresql.org/message-id/28618.1259952...@sss.pgh.pa.us What is to happen if the admin changes the value when the system is already up? If a GUC could be defined as PGC_BACKEND and only settable by superuser, perhaps that would be a good fit. [GucContext seems to conflate some things.] Meanwhile the _init name is meant to convey the fact that it's a before-first-use GUC, like temp_buffers. I'm happy to accept whatever you'd recommend by way of PGC_* GUC selection. Documentation can note any caveats associated with combining plperl.on_perl_init with shared_preload_libraries. http://archives.postgresql.org/message-id/4516.1263168...@sss.pgh.pa.us However, I think PGC_SIGHUP would be enough to address my basic worry, which is that people shouldn't be depending on the ability to set these things within an individual session. The patch uses PGC_SIGHUP for plperl.on_perl_init. http://archives.postgresql.org/message-id/8950.1259994...@sss.pgh.pa.us Tom, what's your objection to Shlib load time being user-visible? It's not really designed to be user-visible. Let me give you just two examples: * We call a plperl function for the first time in a session, causing plperl.so to be loaded. Later the transaction fails and is rolled back. If loading plperl.so caused some user-visible things to happen, should those be rolled back? If so, how do we get perl to play along? If not, how do we get postgres to play along? I believe that's addressed by spi functions being disabled when init code runs. * We call a plperl function for the first time in a session, causing plperl.so to be loaded. This happens in the context of a superuser calling a non-superuser security definer function, or perhaps vice versa. Whose permissions apply to whatever the on_load code tries to do? (Hint: every answer is wrong.) I think that related to on_*trusted_init not plperl.on_perl_init, and is also addressed by spi functions being disabled when init code runs. That doesn't even begin to cover the problems with allowing any of this to happen inside the postmaster. Recall that the postmaster does not have any database access. I believe that's addressed by spi functions being disabled when init code runs. Furthermore, it is a very long established reliability principle around here that the postmaster process should do as little as possible, because every thing that it does creates another opportunity to have a nonrecoverable failure. The postmaster can recover if a child crashes, but the other way round, not so much. I understand that concern. Ultimately, though, that comes down to the judgement of DBAs and the trust placed in them. They can already load arbitrary code via shared_preload_libraries. http://archives.postgresql.org/message-id/18338.1260033...@sss.pgh.pa.us I think if we do this the on_perl_init setting should probably be PGC_POSTMASTER, which would remove any issue about it changing underneath us. Yes, if the main intended usage is in combination with preloading perl at postmaster start, it would be pointless to imagine that PGC_SIGHUP is useful anyway. http://archives.postgresql.org/message-id/17793.1260031...@sss.pgh.pa.us Yeah, in the shower this morning I was thinking that not loading SPI till after the on_init code runs would alleviate the concerns about transactionality and permissions --- that would ensure that whatever on_init does affects only the Perl world and not the database world. That's included
Re: [HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 11:13:43AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Wed, Jan 27, 2010 at 12:46:42AM -0700, Alex Hunsaker wrote: FWIW the atexit scares me to. In what way, specifically? It runs too late, and too unpredictably, during the shutdown sequence. (In particular note that shutdown itself might be fired as an atexit callback, a move forced on us by exactly the sort of random user code that you want to add more of. It's not clear whether a Perl-added atexit would fire before or after that.) man atexit says Functions so registered are called in reverse order. Since the plperl atexit is called only when a plperl SP or DO is executed it would fire before any atexit() registered during startup. The timing and predictability shouldn't be a significant concern if the plperl subsystem can't interact with the rest of the backend - which is the intent. I understand concerns about interacting with the database, so the patch ensures that any use of spi functions throws an exception. That assuages my fears to only a tiny degree. SPI is not the only possible connection between perl code and the rest of the backend. Could you give me some examples of others? Indeed, AFAICS the major *point* of these additions is to allow people to insert unknown other functionality that is likely to interact with the rest of the backend; a prospect that doesn't make me feel better about it. The major point is *not at all* to allow people to interact with the rest of the backend. I'm specifically trying to limit that. The major point is simply to allow perl code to clean itself up properly. Specifically, how is code that starts executing at the end of a session different in risk to code that starts executing before the end of a session? If it runs before the shutdown sequence starts, we know we have a functioning backend. Once shutdown starts, it's unknown and mostly untested exactly what subsystems will still work and which won't. Injecting arbitrary user-written code into an unspecified point in that sequence is not a recipe for good results. The plperl subsystem is isolated from, and can't interact with, the rest of the backend during shutdown. Can you give me examples where that's not the case? Lastly, an atexit trigger will still fire during FATAL or PANIC aborts, which scares me even more. When the house is already afire, it's not prudent to politely let user-written perl code do whatever it wants before you get the heck out of there. Again, that point rests on your underlying concern about interaction between plperl and the rest of the backend. Examples? Is there some way for plperl.c to detect a FATAL or PANIC abort? If so, or if one could be added, then we could skip the END code in those circumstances. I don't really want to add more GUCs, but perhaps controlling END block execution via a plperl.destroy_end=bool (default false) would help address your concerns. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 12:08:48PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Wed, Jan 27, 2010 at 11:13:43AM -0500, Tom Lane wrote: (In particular note that shutdown itself might be fired as an atexit callback, a move forced on us by exactly the sort of random user code that you want to add more of. It's not clear whether a Perl-added atexit would fire before or after that.) man atexit says Functions so registered are called in reverse order. Since the plperl atexit is called only when a plperl SP or DO is executed it would fire before any atexit() registered during startup. Right, which means that it would occur either before or after on_proc_exit processing, depending on whether we got there through an exit() call or via the normal proc_exit sequence. That's just the kind of instability I don't want to have to debug. Okay. I could change the callback code to ignore calls if proc_exit_inprogress is false. So an abnormal shutdown via exit() wouldn't involve plperl at all. (Alternatively I could use use on_proc_exit() instead of atexit() to register the callback.) Would that address this call sequence instability issue? The plperl subsystem is isolated from, and can't interact with, the rest of the backend during shutdown. This is exactly the claim that I have zero confidence in. Quite frankly, the problem with Perl as an extension language is that Perl was never designed to be a subsystem: it feels free to mess around with the entire state of the process. We've been burnt multiple times by that even with the limited use we make of Perl now, and these proposed additions are going to make it a lot worse IMO. On Wed, Jan 27, 2010 at 09:53:44AM -0800, David E. Wheeler wrote: Can you provide an example? Such concerns are impossible to address without concrete examples. On Wed, Jan 27, 2010 at 01:08:56PM -0500, Tom Lane wrote: Two examples that I can find in a quick review of our CVS history: perl stomping on the process's setlocale state, and perl stomping on the stdio state (Windows only). Neither of those relate to the actions of perl source code. To address that, instead of calling perl_destruct() to perform a complete destruction I could just execute END blocks and object destructors. That would avoid executing any system-level actions. Do you have any examples of how a user could write code in a plperl END block that would interact with the rest of the backend? Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 11:28:02AM -0500, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: Indeed, AFAICS the major *point* of these additions is to allow people to insert unknown other functionality that is likely to interact with the rest of the backend; a prospect that doesn't make me feel better about it. No. The major use case we've seen for END blocks is to allow a profiler to write its data out. That should have zero interaction with the rest of the backend. Really? We've found that gprof, for instance, doesn't exactly have zero interaction with the rest of the backend --- there's actually a couple of different bits in there to help it along, including a behavioral change during shutdown. I rather doubt that Perl profilers would turn out much different. Devel::NYTProf (http://blog.timbunce.org/tag/nytprof/) has zero interaction with the rest of the backend. It works in PostgreSQL 8.4, although greatly handicapped by the lack of END blocks. http://search.cpan.org/perldoc?Devel::NYTProf::PgPLPerl But in any case, I don't believe for a moment that profiling is the only or even the largest use to which people would try to put this. Can you give any examples? Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 01:51:47PM -0800, David E. Wheeler wrote:
On Jan 27, 2010, at 1:27 PM, Tim Bunce wrote:
Okay. I could change the callback code to ignore calls if
proc_exit_inprogress is false. So an abnormal shutdown via exit()
wouldn't involve plperl at all. (Alternatively I could use use
on_proc_exit() instead of atexit() to register the callback.)
Given Tom's hesitace about atexit(), perhaps that would be best.
I've a draft patch using !proc_exit_inprogress implemented now
(appended) and I'll test it tomorrow. That was the simplest to do first.
Once I've a reasonable way of testing the exit() and proc_exit() code
paths I'll try using on_proc_exit().
Neither of those relate to the actions of perl source code.
To address that, instead of calling perl_destruct() to perform a
complete destruction I could just execute END blocks and object
destructors. That would avoid executing any system-level actions.
Does perl_destruct() execute system-level actions, then?
Potentially: Kills threads it knows about (should be none), wait for
children (should be none), flushes all open *perl* file handles (should
be none for plperl), tears down PerlIO layers, etc. etc. In practice
none of that should affect the backend, but it's possible, especially
for the Windows port. Since none of that is needed it can be skipped.
If so, then it seems prudent to either audit such actions or, as you
say, call destructors directly.
The patch now just calls END blocks and DESTROY methods.
Do you have any examples of how a user could write code in a plperl END
block that would interact with the rest of the backend?
I appreciate you taking the time to look at ways to address the issues
Tom has raised, Tim. Good on you.
Thanks David. I appreciate the visible support!
Tom and the team set the bar high, rightly, so it's certainly been a
challenging introduction to PostgreSQL development!
Tim.
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 8315d5a..38f2d35 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***
*** 27,32
--- 27,33
#include miscadmin.h
#include nodes/makefuncs.h
#include parser/parse_type.h
+ #include storage/ipc.h
#include utils/builtins.h
#include utils/fmgroids.h
#include utils/guc.h
*** _PG_init(void)
*** 281,287
static void
plperl_fini(void)
{
! plperl_ending = true;
plperl_destroy_interp(plperl_trusted_interp);
plperl_destroy_interp(plperl_untrusted_interp);
plperl_destroy_interp(plperl_held_interp);
--- 282,297
static void
plperl_fini(void)
{
! plperl_ending = true; /* disables use of spi_* functions */
!
! /*
!* Only perform perl cleanup if we're exiting cleanly via proc_exit().
!* If proc_exit_inprogress is false then exit() was called directly
!* (because we call atexit() very late, so get called early).
!*/
! if (!proc_exit_inprogress)
! return;
!
plperl_destroy_interp(plperl_trusted_interp);
plperl_destroy_interp(plperl_untrusted_interp);
plperl_destroy_interp(plperl_held_interp);
*** plperl_destroy_interp(PerlInterpreter **
*** 595,602
{
if (interp *interp)
{
! perl_destruct(*interp);
! perl_free(*interp);
*interp = NULL;
}
}
--- 605,640
{
if (interp *interp)
{
! /*
!* Only a very minimal destruction is performed.
!* Just END blocks and object destructors, no system-level
actions.
!* Code code here extracted from perl's perl_destruct().
!*/
!
! /* Run END blocks */
! if (PL_exit_flags PERL_EXIT_DESTRUCT_END) {
! dJMPENV;
! int x = 0;
!
! JMPENV_PUSH(x);
! PERL_UNUSED_VAR(x);
! if (PL_endav !PL_minus_c)
! call_list(PL_scopestack_ix, PL_endav);
! JMPENV_POP;
! }
! LEAVE;
! FREETMPS;
!
! PL_dirty = TRUE;
!
! /* destroy objects - call DESTROY methods */
! if (PL_sv_objcount) {
! Perl_sv_clean_objs(aTHX);
! PL_sv_objcount = 0;
! if (PL_defoutgv !SvREFCNT(PL_defoutgv))
! PL_defoutgv = NULL; /* may have been freed */
! }
!
*interp = NULL;
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Miscellaneous changes to plperl [PATCH]
On Sat, Jan 23, 2010 at 06:40:03PM -0700, Alex Hunsaker wrote:
On Sat, Jan 23, 2010 at 16:16, Tim Bunce tim.bu...@pobox.com wrote:
On Fri, Jan 22, 2010 at 08:59:10PM -0700, Alex Hunsaker wrote:
On Thu, Jan 14, 2010 at 09:07, Tim Bunce tim.bu...@pobox.com wrote:
I'd vote for use warnings; as well.
I would to, but sadly it's not that simple.
warnings uses Carp and Carp uses eval { ... } and, owing to a sad bug in
perl 5.11.4, Safe can't distinguish between eval ... and eval {...}
http://rt.perl.org/rt3/Ticket/Display.html?id=70970
So trying to load warnings fails (at least for some versions of perl).
Well that stinks.
Yeap. I was amazed that no one had run into it before.
I have a version of my final Package namespace and Safe init cleanup
for plperl that works around that. I opted to post a less potentially
controversial version of that patch in the end. If you think allowing
plperl code to 'use warnings;' is important (and I'd tend to agree)
then I'll update that final patch.
Sounds good.
FYI I've an updated patch ready but I'll wait till the commitfest has
got 'closer' as there's a fair chance a further update will be needed
anyway to make a patch that applies cleanly.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Miscellaneous changes to plperl [PATCH]
On Mon, Jan 25, 2010 at 11:09:12AM -0500, Andrew Dunstan wrote: Tim Bunce wrote: FYI I've an updated patch ready but I'll wait till the commitfest has got 'closer' as there's a fair chance a further update will be needed anyway to make a patch that applies cleanly. I want to deal with this today or tomorrow, so don't sit on it, please. Okay. I'll post it as a reply to the original and add it to the commitfest. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]
On Fri, Jan 15, 2010 at 04:02:02AM +, Tim Bunce wrote:
This is the final plperl patch in the series from me.
Changes in this patch:
- Moved internal functions out of main:: namespace
into PostgreSQL::InServer and PostgreSQL::InServer::safe
- Restructured Safe compartment setup code
to generalize and separate the data from the logic.
Neither change has any user visible effects.
This is a revised version of the patch with the following additional
changes:
- Further generalized the 'what to load into Safe compartment' logic.
- Added the 'warnings' pragma to the list of modules to load into Safe.
So plperl functions can now use warnings; - added test for that.
- Added 'use 5.008001;' to plc_perlboot.pl as a run-time check to
complement the configure-time check added by Tom Lane recently.
Tim.
diff --git a/src/pl/plperl/expected/plperl.out b/src/pl/plperl/expected/plperl.out
index ebf9afd..0e7c65d 100644
*** a/src/pl/plperl/expected/plperl.out
--- b/src/pl/plperl/expected/plperl.out
*** CONTEXT: PL/Perl anonymous code block
*** 577,579
--- 577,584
DO $do$ use strict; my $name = foo; my $ref = $$name; $do$ LANGUAGE plperl;
ERROR: Can't use string (foo) as a SCALAR ref while strict refs in use at line 1.
CONTEXT: PL/Perl anonymous code block
+ -- check that we can use warnings (in this case to turn a warn into an error)
+ -- yields ERROR: Useless use of length in void context
+ DO $do$ use warnings FATAL = qw(void) ; length abc ; 1; $do$ LANGUAGE plperl;
+ ERROR: Useless use of length in void context at line 1.
+ CONTEXT: PL/Perl anonymous code block
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index 5f6ae91..239456c 100644
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 1,23
PostgreSQL::InServer::Util::bootstrap();
use strict;
use warnings;
use vars qw(%_SHARED);
! sub ::plperl_warn {
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
! elog(NOTICE, $msg);
}
! $SIG{__WARN__} = \::plperl_warn;
! sub ::plperl_die {
(my $msg = shift) =~ s/\(eval \d+\) //g;
die $msg;
}
! $SIG{__DIE__} = \::plperl_die;
! sub ::mkfuncsrc {
my ($name, $imports, $prolog, $src) = @_;
my $BEGIN = join \n, map {
--- 1,27
+ use 5.008001;
+
PostgreSQL::InServer::Util::bootstrap();
+ package PostgreSQL::InServer;
+
use strict;
use warnings;
use vars qw(%_SHARED);
! sub plperl_warn {
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
! ::elog(::NOTICE, $msg);
}
! $SIG{__WARN__} = \plperl_warn;
! sub plperl_die {
(my $msg = shift) =~ s/\(eval \d+\) //g;
die $msg;
}
! $SIG{__DIE__} = \plperl_die;
! sub mkfuncsrc {
my ($name, $imports, $prolog, $src) = @_;
my $BEGIN = join \n, map {
*** sub ::mkfuncsrc {
*** 30,44
$name =~ s/::|'/_/g; # avoid package delimiters
my $funcsrc;
! $funcsrc .= qq[ undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
#warn plperl mkfuncsrc: $funcsrc\n;
return $funcsrc;
}
# see also mksafefunc() in plc_safe_ok.pl
! sub ::mkunsafefunc {
no strict; # default to no strict for the eval
! my $ret = eval(::mkfuncsrc(@_));
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
--- 34,48
$name =~ s/::|'/_/g; # avoid package delimiters
my $funcsrc;
! $funcsrc .= qq[ package main; undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
#warn plperl mkfuncsrc: $funcsrc\n;
return $funcsrc;
}
# see also mksafefunc() in plc_safe_ok.pl
! sub mkunsafefunc {
no strict; # default to no strict for the eval
! my $ret = eval(mkfuncsrc(@_));
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
*** sub ::encode_array_literal {
*** 67,73
sub ::encode_array_constructor {
my $arg = shift;
! return quote_nullable($arg)
if ref $arg ne 'ARRAY';
my $res = join , , map {
(ref $_) ? ::encode_array_constructor($_)
--- 71,77
sub ::encode_array_constructor {
my $arg = shift;
! return ::quote_nullable($arg)
if ref $arg ne 'ARRAY';
my $res = join , , map {
(ref $_) ? ::encode_array_constructor($_)
diff --git a/src/pl/plperl/plc_safe_ok.pl b/src/pl/plperl/plc_safe_ok.pl
index 7b36e33..7dc330e 100644
*** a/src/pl/plperl/plc_safe_ok.pl
--- b/src/pl/plperl/plc_safe_ok.pl
***
*** 1,39
use strict;
! use vars qw($PLContainer);
- $PLContainer = new Safe('PLPerl');
$PLContainer-permit_only(':default');
$PLContainer-permit(qw[:base_math !:base_io sort time require]);
- $PLContainer-share(qw[elog return_next
- spi_query spi_fetchrow spi_cursor_close spi_exec_query
- spi_prepare spi_exec_prepared spi_query_prepared spi_freeplan
- DEBUG LOG INFO NOTICE WARNING ERROR %_SHARED
- quote_literal quote_nullable quote_ident
- encode_bytea decode_bytea
- encode_array_literal encode_array_constructor
- looks_like_number
- ]);
! # Load
Re: [HACKERS] Miscellaneous changes to plperl [PATCH]
On Fri, Jan 22, 2010 at 08:59:10PM -0700, Alex Hunsaker wrote:
On Thu, Jan 14, 2010 at 09:07, Tim Bunce tim.bu...@pobox.com wrote:
- Allow (ineffective) use of 'require' in plperl
If the required module is not already loaded then it dies.
So use strict; now works in plperl.
[ BTW I think this is awesome! ]
Thanks!
I'd vote for use warnings; as well.
I would to, but sadly it's not that simple.
warnings uses Carp and Carp uses eval { ... } and, owing to a sad bug in
perl 5.11.4, Safe can't distinguish between eval ... and eval {...}
http://rt.perl.org/rt3/Ticket/Display.html?id=70970
So trying to load warnings fails (at least for some versions of perl).
I have a version of my final Package namespace and Safe init cleanup
for plperl that works around that. I opted to post a less potentially
controversial version of that patch in the end. If you think allowing
plperl code to 'use warnings;' is important (and I'd tend to agree)
then I'll update that final patch.
- Stored procedure subs are now given names.
The names are not visible in ordinary use, but they make
tools like Devel::NYTProf and Devel::Cover _much_ more useful.
This needs to quote at least '. Any others you can think of? Also I
think we should sort the imports in ::mkfunsort so that they are
stable.
Sort for stability, yes. The quoting is fine though (I see you've come
to the same conclusion via David).
The cleanups were nice, the code worked as described.
Thanks.
Other than the quoting issue it looks good to me. Find below an
incremental patch that fixes the items above.
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index daef469..fa5df0a 100644
--- a/src/pl/plperl/plc_perlboot.pl
+++ b/src/pl/plperl/plc_perlboot.pl
@@ -27,16 +27,14 @@ sub ::mkfuncsrc {
my $BEGIN = join \n, map {
my $names = $imports-{$_} || [];
$_-import(qw(@$names));
- } keys %$imports;
+ } sort keys %$imports;
Ok, good.
$name =~ s/\\//g;
$name =~ s/::|'/_/g; # avoid package delimiters
+ $name =~ s/'/\'/g;
Not needed.
- my $funcsrc;
- $funcsrc .= qq[ undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src
} ];
- #warn plperl mkfuncsrc: $funcsrc\n;
- return $funcsrc;
+ return qq[ undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
}
Ok. (I don't think that'll clash with any later patches.)
# see also mksafefunc() in plc_safe_ok.pl
diff --git a/src/pl/plperl/plc_safe_ok.pl b/src/pl/plperl/plc_safe_ok.pl
index 8d35357..79d64ce 100644
--- a/src/pl/plperl/plc_safe_ok.pl
+++ b/src/pl/plperl/plc_safe_ok.pl
@@ -25,6 +25,7 @@ $PLContainer-share(qw[elog return_next
$PLContainer-permit(qw[caller]);
::safe_eval(q{
require strict;
+ require warnings;
require feature if $] = 5.01;
1;
}) or die $@;
Not viable, sadly.
On Sat, Jan 23, 2010 at 12:42, David E. Wheeler da...@kineticode.com wrote:
On Jan 23, 2010, at 11:20 AM, Alex Hunsaker wrote:
Well no, i suppose we could fix that via:
$name =~ s/[:|']/_/g;
Im betting that was the intent.
Doubtful. In Perl, the package separator is either `::` or `'` (for
hysterical reasons). So the original code was replacing any package
separator with a single underscore. Your regex would change This::Module to
This__Module, which I'm certain was not the intent.
Haha, yep your right. I could have sworn I tested it with a function
name with a ' and it broke. But your obviously right :)
I could have sworn I wrote a test file with a bunch of stressful names.
All seems well though:
template1=# create or replace function a'b*c}d!() returns text language
plperl as '42';
CREATE FUNCTION
template1=# select a'b*c}d!();
a'b*c}d!
--
42
So, what now? Should I resend the patch with the two 'ok' changes above
included, or can the committer make those very minor changes?
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warn in plperl logs as... NOTICE??
On Thu, Jan 21, 2010 at 07:55:09PM -0500, Andrew Dunstan wrote:
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Jim Nasby wrote:
Why does warn; in plperl log as NOTICE in Postgres?
Where would you like the warning to go? This has been this way
for nearly 5 years, it's not new (and before that the warning
didn't go anywhere).
I think he's suggesting that it ought to translate as elog(WARNING)
not elog(NOTICE).
*shrug* I don't have a strong opinion about it, and it's pretty easy
to change, if there's a consensus we should. I have certainly found
over the years that perl warnings from some modules can be
annoyingly verbose, which is probably why the original patch didn't
make them have a higher level in Postgres. If this were a big issue
we'd have surely heard about it before now - there are plenty of
plperl users out there.
I've no particular opinion either way on this. I can't resist the
tempation, however, to point out that this is an example the kind of
site-preference that could be handled via plperl.on_perl_init:
plperl.on_perl_init='$SIG{__WARN__} = sub { elog(WARNING, shift) }'
or
plperl.on_perl_init='use lib /MyApp/lib; use MyApp::PLPerlInit;'
You could get more fancy and employ some logic to using WARNING for the
first instance of any given message text and NOTICE for subsequent ones.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Helping the CommitFest re PL/Perl changes
What can I do to help the CommitFest, especially in relation to the PL/Perl changes? Tim. p.s. I've sent an email to the dbi-users and dbi-announce lists http://www.mail-archive.com/dbi-us...@perl.org/msg32649.html in the hope of encouraging some more people to review and test the patches, and hopefully contribute further to this and future CommitFests. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Miscellaneous changes to plperl [PATCH]
This is the second of the patches to be split out from the former
'plperl feature patch 1'.
Changes in this patch:
- Allow (ineffective) use of 'require' in plperl
If the required module is not already loaded then it dies.
So use strict; now works in plperl.
- Pre-load the feature module if perl = 5.10.
So use feature :5.10; now works in plperl.
- Stored procedure subs are now given names.
The names are not visible in ordinary use, but they make
tools like Devel::NYTProf and Devel::Cover _much_ more useful.
- Simplified and generalized the subroutine creation code.
Now one code path for generating sub source code, not four.
Can generate multiple 'use' statements with specific imports
(which handles plperl.use_strict currently and can easily
be extended to handle a plperl.use_feature=':5.12' in future).
- Disallows use of Safe version 2.20 which is broken for PL/Perl.
http://rt.perl.org/rt3/Ticket/Display.html?id=72068
- Assorted minor optimizations by pre-growing data structures.
This patch will apply cleanly over the 'add functions' patch:
https://commitfest.postgresql.org/action/patch_view?id=264
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 94db722..6fee031 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** SELECT * FROM perl_set();
*** 285,313
/para
para
!If you wish to use the literalstrict/ pragma with your code,
!the easiest way to do so is to commandSET/
!literalplperl.use_strict/literal to true. This parameter affects
!subsequent compilations of applicationPL/Perl/ functions, but not
!functions already compiled in the current session. To set the
!parameter before applicationPL/Perl/ has been loaded, it is
!necessary to have added quoteliteralplperl// to the xref
!linkend=guc-custom-variable-classes list in
!filenamepostgresql.conf/filename.
/para
para
!Another way to use the literalstrict/ pragma is to put:
programlisting
use strict;
/programlisting
!in the function body. But this only works in applicationPL/PerlU/
!functions, since the literaluse/ triggers a literalrequire/
!which is not a trusted operation. In
!applicationPL/Perl/ functions you can instead do:
! programlisting
! BEGIN { strict-import(); }
! /programlisting
/para
/sect1
--- 285,323
/para
para
!If you wish to use the literalstrict/ pragma with your code you have a few options.
!For temporary global use you can commandSET/ literalplperl.use_strict/literal
!to true (see xref linkend=plperl.use_strict).
!This will affect subsequent compilations of applicationPL/Perl/
!functions, but not functions already compiled in the current session.
!For permanent global use you can set literalplperl.use_strict/literal
!to true in the filenamepostgresql.conf/filename file.
/para
para
!For permanent use in specific functions you can simply put:
programlisting
use strict;
/programlisting
!at the top of the function body.
! /para
!
! para
! The literalfeature/ pragma is also available to functionuse/ if your Perl is version 5.10.0 or higher.
! /para
!
! /sect1
!
! sect1 id=plperl-data
! titleData Values in PL/Perl/title
!
! para
!The argument values supplied to a PL/Perl function's code are
!simply the input arguments converted to text form (just as if they
!had been displayed by a commandSELECT/command statement).
!Conversely, the functionreturn/function and functionreturn_next/function
!commands will accept any string that is acceptable input format
!for the function's declared return type.
/para
/sect1
*** SELECT done();
*** 682,699
/sect2
/sect1
- sect1 id=plperl-data
- titleData Values in PL/Perl/title
-
- para
-The argument values supplied to a PL/Perl function's code are
-simply the input arguments converted to text form (just as if they
-had been displayed by a commandSELECT/command statement).
-Conversely, the literalreturn/ command will accept any string
-that is acceptable input format for the function's declared return
-type. So, within the PL/Perl function,
-all values are just text strings.
- /para
/sect1
sect1 id=plperl-global
--- 692,697
*** CREATE TRIGGER test_valid_id_trig
*** 1042,1049
itemizedlist
listitem
para
! PL/Perl functions cannot call each other directly (because they
! are anonymous subroutines inside Perl).
/para
/listitem
--- 1040,1046
itemizedlist
listitem
para
! PL/Perl functions cannot call each other directly.
/para
/listitem
*** CREATE TRIGGER test_valid_id_trig
*** 1072,1077
--- 1069,1076
/listitem
/itemizedlist
/para
+ /sect2
+
/sect1
/chapter
diff
Re: [HACKERS] Miscellaneous changes to plperl [PATCH]
On Thu, Jan 14, 2010 at 09:34:42AM -0800, David E. Wheeler wrote: On Jan 14, 2010, at 8:07 AM, Tim Bunce wrote: - Stored procedure subs are now given names. The names are not visible in ordinary use, but they make tools like Devel::NYTProf and Devel::Cover _much_ more useful. Wasn't this in the previous patch, too? Ah, I see it was in the description of the previous patch but not in the patch itself. Thanks. I'll add a note to the commitfest. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Add on_perl_init and proper destruction to plperl [PATCH]
This is the third of the patches to be split out from the former 'plperl
feature patch 1'.
Changes in this patch:
- Added plperl.on_perl_init GUC for DBA use (PGC_SIGHUP)
SPI functions are not available when the code is run.
- Added normal interpreter destruction behaviour
END blocks, if any, are run then objects are
destroyed, calling their DESTROY methods, if any.
SPI functions will die if called at this time.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 6fee031..0054f5a 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** CREATE TRIGGER test_valid_id_trig
*** 1030,1036
/para
/sect1
! sect1 id=plperl-missing
titleLimitations and Missing Features/title
para
--- 1030,1100
/para
/sect1
! sect1 id=plperl-under-the-hood
! titlePL/Perl Under the Hood/title
!
! sect2 id=plperl-config
! titleConfiguration/title
!
! para
! This section lists configuration parameters that affect applicationPL/Perl/.
! To set any of these parameters before applicationPL/Perl/ has been loaded,
! it is necessary to have added quoteliteralplperl// to the
! xref linkend=guc-custom-variable-classes list in
! filenamepostgresql.conf/filename.
! /para
!
! variablelist
!
! varlistentry id=guc-plperl-on-perl-init xreflabel=plperl.on_perl_init
! termvarnameplperl.on_perl_init/varname (typestring/type)/term
! indexterm
!primaryvarnameplperl.on_perl_init/ configuration parameter/primary
! /indexterm
! listitem
!para
!Specifies perl code to be executed when a perl interpreter is first initialized.
!The SPI functions are not available when this code is executed.
!If the code fails with an error it will abort the initialization of the interpreter
!and propagate out to the calling query, causing the current transaction
!or subtransaction to be aborted.
!/para
!para
! The perl code is limited to a single string. Longer code can be placed
! into a module and loaded by the literalon_perl_init/ string.
! Examples:
! programlisting
! plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;'
! /programlisting
!/para
!para
!Initialization will happen in the postmaster if the plperl library is included
!in literalshared_preload_libraries/ (see xref linkend=shared_preload_libraries),
!in which case extra consideration should be given to the risk of destabilizing the postmaster.
!/para
!para
!This parameter can only be set in the postgresql.conf file or on the server command line.
!/para
! /listitem
! /varlistentry
!
! varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict
! termvarnameplperl.use_strict/varname (typeboolean/type)/term
! indexterm
!primaryvarnameplperl.use_strict/ configuration parameter/primary
! /indexterm
! listitem
!para
!When set true subsequent compilations of PL/Perl functions have the literalstrict/ pragma enabled.
!This parameter does not affect functions already compiled in the current session.
!/para
! /listitem
! /varlistentry
!
! /variablelist
!
! sect2 id=plperl-missing
titleLimitations and Missing Features/title
para
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index 769721d..5f6ae91 100644
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 1,5
PostgreSQL::InServer::Util::bootstrap();
- PostgreSQL::InServer::SPI::bootstrap();
use strict;
use warnings;
--- 1,4
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 9277072..8315d5a 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*** static HTAB *plperl_proc_hash = NULL;
*** 138,143
--- 138,145
static HTAB *plperl_query_hash = NULL;
static bool plperl_use_strict = false;
+ static char *plperl_on_perl_init = NULL;
+ static bool plperl_ending = false;
/* this is saved and restored by plperl_call_handler */
static plperl_call_data *current_call_data = NULL;
*** Datum plperl_validator(PG_FUNCTION_ARGS
*** 151,156
--- 153,160
void _PG_init(void);
static PerlInterpreter *plperl_init_interp(void);
+ static void plperl_destroy_interp(PerlInterpreter **);
+ static void plperl_fini(void);
static Datum plperl_func_handler(PG_FUNCTION_ARGS);
static Datum plperl_trigger_handler(PG_FUNCTION_ARGS);
*** _PG_init(void)
*** 237,242
--- 241,254
PGC_USERSET, 0,
NULL, NULL);
+ DefineCustomStringVariable(plperl.on_perl_init,
+ gettext_noop(Perl code to execute when the perl interpreter is initialized.),
+ NULL,
+
[HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
This is the fourth of the patches to be split out from the former
'plperl feature patch 1'.
Changes in this patch:
- Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs
Both are PGC_USERSET.
SPI functions are not available when the code is run.
Errors are detected and reported as ereport(ERROR, ...)
- select_perl_context() state management improved
An error during interpreter initialization will leave
the state (interp_state etc) unchanged.
- The utf8fix code has been greatly simplified.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 0054f5a..f2c91a9 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** plplerl.on_perl_init = 'use lib /my/app
*** 1079,1084
--- 1079,1120
/listitem
/varlistentry
+ varlistentry id=guc-plperl-on-trusted-init xreflabel=plperl.on_trusted_init
+ termvarnameplperl.on_trusted_init/varname (typestring/type)/term
+ indexterm
+primaryvarnameplperl.on_trusted_init/ configuration parameter/primary
+ /indexterm
+ listitem
+para
+Specifies perl code to be executed when the literalplperl/ perl interpreter
+is first initialized in a session. The perl code can only perform trusted operations.
+The SPI functions are not available when this code is executed.
+Changes made after a literalplperl/ perl interpreter has been initialized will have no effect.
+If the code fails with an error it will abort the initialization of the interpreter
+and propagate out to the calling query, causing the current transaction
+or subtransaction to be aborted.
+/para
+ /listitem
+ /varlistentry
+
+ varlistentry id=guc-plperl-on-untrusted-init xreflabel=plperl.on_untrusted_init
+ termvarnameplperl.on_untrusted_init/varname (typestring/type)/term
+ indexterm
+primaryvarnameplperl.on_untrusted_init/ configuration parameter/primary
+ /indexterm
+ listitem
+para
+Specifies perl code to be executed when the literalplperlu/ perl interpreter
+is first initialized in a session.
+The SPI functions are not available when this code is executed.
+Changes made after a literalplperlu/ perl interpreter has been initialized will have no effect.
+If the code fails with an error it will abort the initialization of the interpreter
+and propagate out to the calling query, causing the current transaction
+or subtransaction to be aborted.
+/para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict
termvarnameplperl.use_strict/varname (typeboolean/type)/term
indexterm
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index 7cd5721..f3cabad 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*** PERLCHUNKS = plc_perlboot.pl plc_safe_ba
*** 41,47
SHLIB_LINK = $(perl_embed_ldflags)
REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperlu
# if Perl can support two interpreters in one backend,
# test plperl-and-plperlu cases
ifneq ($(PERL),)
--- 41,47
SHLIB_LINK = $(perl_embed_ldflags)
REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperl_init plperlu
# if Perl can support two interpreters in one backend,
# test plperl-and-plperlu cases
ifneq ($(PERL),)
diff --git a/src/pl/plperl/expected/plperl_init.out b/src/pl/plperl/expected/plperl_init.out
index ...e69de29 .
diff --git a/src/pl/plperl/expected/plperl_shared.out b/src/pl/plperl/expected/plperl_shared.out
index 72ae1ba..c1c12c1 100644
*** a/src/pl/plperl/expected/plperl_shared.out
--- b/src/pl/plperl/expected/plperl_shared.out
***
*** 1,3
--- 1,7
+ -- test plperl.on_plperl_init via the shared hash
+ -- (must be done before plperl is initialized)
+ -- testing on_trusted_init gets run, and that it can alter %_SHARED
+ SET plperl.on_trusted_init = '$_SHARED{on_init} = 42';
-- test the shared hash
create function setme(key text, val text) returns void language plperl as $$
*** select getme('ourkey');
*** 24,26
--- 28,36
ourval
(1 row)
+ select getme('on_init');
+ getme
+ ---
+ 42
+ (1 row)
+
diff --git a/src/pl/plperl/plc_safe_ok.pl b/src/pl/plperl/plc_safe_ok.pl
index dc33dd6..7b36e33 100644
*** a/src/pl/plperl/plc_safe_ok.pl
--- b/src/pl/plperl/plc_safe_ok.pl
*** $PLContainer-permit(qw[caller]);
*** 27,32
--- 27,33
}) or die $@;
$PLContainer-deny(qw[caller]);
+ # called directly for plperl.on_trusted_init
sub ::safe_eval {
my $ret = $PLContainer-reval(shift);
$@ =~
[HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]
This is the final plperl patch in the series from me.
Changes in this patch:
- Moved internal functions out of main:: namespace
into PostgreSQL::InServer and PostgreSQL::InServer::safe
- Restructured Safe compartment setup code
to generalize and separate the data from the logic.
Neither change has any user visible effects.
This patch will apply cleanly over the 'Add on_trusted_init and
on_untrusted_init to plperl' patch:
https://commitfest.postgresql.org/action/patch_view?id=271
Tim.
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index 5f6ae91..05ed049 100644
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 1,23
PostgreSQL::InServer::Util::bootstrap();
use strict;
use warnings;
use vars qw(%_SHARED);
! sub ::plperl_warn {
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
! elog(NOTICE, $msg);
}
! $SIG{__WARN__} = \::plperl_warn;
! sub ::plperl_die {
(my $msg = shift) =~ s/\(eval \d+\) //g;
die $msg;
}
! $SIG{__DIE__} = \::plperl_die;
! sub ::mkfuncsrc {
my ($name, $imports, $prolog, $src) = @_;
my $BEGIN = join \n, map {
--- 1,25
PostgreSQL::InServer::Util::bootstrap();
+ package PostgreSQL::InServer;
+
use strict;
use warnings;
use vars qw(%_SHARED);
! sub plperl_warn {
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
! ::elog(::NOTICE, $msg);
}
! $SIG{__WARN__} = \plperl_warn;
! sub plperl_die {
(my $msg = shift) =~ s/\(eval \d+\) //g;
die $msg;
}
! $SIG{__DIE__} = \plperl_die;
! sub mkfuncsrc {
my ($name, $imports, $prolog, $src) = @_;
my $BEGIN = join \n, map {
*** sub ::mkfuncsrc {
*** 30,44
$name =~ s/::|'/_/g; # avoid package delimiters
my $funcsrc;
! $funcsrc .= qq[ undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
#warn plperl mkfuncsrc: $funcsrc\n;
return $funcsrc;
}
# see also mksafefunc() in plc_safe_ok.pl
! sub ::mkunsafefunc {
no strict; # default to no strict for the eval
! my $ret = eval(::mkfuncsrc(@_));
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
--- 32,46
$name =~ s/::|'/_/g; # avoid package delimiters
my $funcsrc;
! $funcsrc .= qq[ package main; undef *{'$name'}; *{'$name'} = sub { $BEGIN $prolog $src } ];
#warn plperl mkfuncsrc: $funcsrc\n;
return $funcsrc;
}
# see also mksafefunc() in plc_safe_ok.pl
! sub mkunsafefunc {
no strict; # default to no strict for the eval
! my $ret = eval(mkfuncsrc(@_));
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
*** sub ::encode_array_literal {
*** 67,73
sub ::encode_array_constructor {
my $arg = shift;
! return quote_nullable($arg)
if ref $arg ne 'ARRAY';
my $res = join , , map {
(ref $_) ? ::encode_array_constructor($_)
--- 69,75
sub ::encode_array_constructor {
my $arg = shift;
! return ::quote_nullable($arg)
if ref $arg ne 'ARRAY';
my $res = join , , map {
(ref $_) ? ::encode_array_constructor($_)
diff --git a/src/pl/plperl/plc_safe_ok.pl b/src/pl/plperl/plc_safe_ok.pl
index 7b36e33..fcf5d54 100644
*** a/src/pl/plperl/plc_safe_ok.pl
--- b/src/pl/plperl/plc_safe_ok.pl
***
*** 1,39
use strict;
! use vars qw($PLContainer);
- $PLContainer = new Safe('PLPerl');
$PLContainer-permit_only(':default');
$PLContainer-permit(qw[:base_math !:base_io sort time require]);
- $PLContainer-share(qw[elog return_next
- spi_query spi_fetchrow spi_cursor_close spi_exec_query
- spi_prepare spi_exec_prepared spi_query_prepared spi_freeplan
- DEBUG LOG INFO NOTICE WARNING ERROR %_SHARED
- quote_literal quote_nullable quote_ident
- encode_bytea decode_bytea
- encode_array_literal encode_array_constructor
- looks_like_number
- ]);
-
- # Load widely useful pragmas into the container to make them available.
# (Temporarily enable caller here as work around for bug in perl 5.10,
# which changed the way its Safe.pm works. It is quite safe, as caller is
# informational only.)
$PLContainer-permit(qw[caller]);
! ::safe_eval(q{
! require strict;
! require feature if $] = 5.01;
! 1;
! }) or die $@;
$PLContainer-deny(qw[caller]);
# called directly for plperl.on_trusted_init
! sub ::safe_eval {
my $ret = $PLContainer-reval(shift);
$@ =~ s/\(eval \d+\) //g if $@;
return $ret;
}
! sub ::mksafefunc {
! return ::safe_eval(::mkfuncsrc(@_));
}
--- 1,56
+ package PostgreSQL::InServer::safe;
+
use strict;
! use warnings;
!
! use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);
!
! # Load widely useful pragmas into the container to make them available.
! # These must be trusted to not expose a way to execute a string eval
! # or any kind of unsafe action that the untrusted code could exploit.
! # If in any doubt about a module then do not add it to this list.
! push @EvalInSafe, 'require feature' if $] = 5.01;
! push @EvalInSafe,
Re: [HACKERS] Feature patch 1 for plperl [PATCH]
On Tue, Jan 12, 2010 at 07:06:59PM -0500, Robert Haas wrote: On Sun, Jan 10, 2010 at 4:35 PM, Robert Haas robertmh...@gmail.com wrote: I would strongly suggest to Tim that he rip the portions of this patch that are related to this feature out and submit them separately so that we can commit the uncontroversial portions first. See my previous email. I suggested that Tim send three patches: one for this controversial stuff, one for the new utility functions for plperl, and one for the remainder. He and I have discussed it and I believe he is agreeable to that. OK, well then just +1 for that. I believe we have agreement on this course of action, so I'm going to mark the current patch as Returned with Feedback. Hopefully Tim will submit separate patches for each of these three areas in the next day or two before start-of-CommitFest That's my plan. Plus, hopefully at least one more for inter-sp calling. personally, I think they should each get their own thread and their own entry in the CommitFest app, for ease of tracking and reviewing. YMMV, of course. Yes, that was also my intent. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Add utility functions to plperl [PATCH]
This is the first of the patches to be split out from the former 'plperl
feature patch 1'.
Changes in this patch:
- Added utility functions: quote_literal, quote_nullable, quote_ident,
encode_bytea, decode_bytea, looks_like_number,
encode_array_literal, encode_array_constructor.
- Stored procedure subs are now given names ($name__$oid).
This is invisible to PL/Perl stored procedures but makes
tools like Devel::NYTProf and Devel::Cover _much_ more useful.
- Warnings no longer have an extra newline in the NOTICE text.
- Corresponding documentation changes in plperl.sgml, plus:
Removed some trailing whitespace.
Made some examples use more idiomatic perl.
Added the missing ', arguments' to docs of spi_exec_prepared().
- Assorted minor changes
Various minor optimizations like pre-growing data structures.
Makes proper use of the recently updated ppport.h.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 37114bd..94db722 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
***
*** 13,19
para
PL/Perl is a loadable procedural language that enables you to write
!productnamePostgreSQL/productname functions in the
ulink url=http://www.perl.org;Perl programming language/ulink.
/para
--- 13,19
para
PL/Perl is a loadable procedural language that enables you to write
!productnamePostgreSQL/productname functions in the
ulink url=http://www.perl.org;Perl programming language/ulink.
/para
*** $$ LANGUAGE plperl;
*** 101,107
linkend=sql-syntax-dollar-quoting) for the string constant.
If you choose to use escape string syntax literalE''/,
you must double the single quote marks (literal'/) and backslashes
!(literal\/) used in the body of the function
(see xref linkend=sql-syntax-strings).
/para
--- 101,107
linkend=sql-syntax-dollar-quoting) for the string constant.
If you choose to use escape string syntax literalE''/,
you must double the single quote marks (literal'/) and backslashes
!(literal\/) used in the body of the function
(see xref linkend=sql-syntax-strings).
/para
*** $$ LANGUAGE plperl;
*** 141,153
programlisting
CREATE FUNCTION perl_max (integer, integer) RETURNS integer AS $$
! my ($x,$y) = @_;
! if (! defined $x) {
! if (! defined $y) { return undef; }
return $y;
}
! if (! defined $y) { return $x; }
! if ($x gt; $y) { return $x; }
return $y;
$$ LANGUAGE plperl;
/programlisting
--- 141,153
programlisting
CREATE FUNCTION perl_max (integer, integer) RETURNS integer AS $$
! my ($x, $y) = @_;
! if (not defined $x) {
! return undef if not defined $y;
return $y;
}
! return $x if not defined $y;
! return $x if $x gt; $y;
return $y;
$$ LANGUAGE plperl;
/programlisting
*** $$ LANGUAGE plperl;
*** 158,189
para
Anything in a function argument that is not a reference is
!a string, which is in the standard productnamePostgreSQL/productname
external text representation for the relevant data type. In the case of
ordinary numeric or text types, Perl will just do the right thing and
the programmer will normally not have to worry about it. However, in
!other cases the argument will need to be converted into a form that is
!more usable in Perl. For example, here is how to convert an argument of
!type typebytea/ into unescaped binary
!data:
!
! programlisting
! my $arg = shift;
! $arg =~ s!\\(?:\\|(\d{3}))!$1 ? chr(oct($1)) : \\!ge;
! /programlisting
!
/para
para
!Similarly, values passed back to productnamePostgreSQL/productname
!must be in the external text representation format. For example, here
!is how to escape binary data for a return value of type typebytea/:
!
! programlisting
! $retval =~ s!(\\|[^ -~])!sprintf(\\%03o,ord($1))!ge;
! return $retval;
! /programlisting
!
/para
para
--- 158,178
para
Anything in a function argument that is not a reference is
!a string, which is in the standard productnamePostgreSQL/productname
external text representation for the relevant data type. In the case of
ordinary numeric or text types, Perl will just do the right thing and
the programmer will normally not have to worry about it. However, in
!other cases the argument will need to be converted into a form that is
!more usable in Perl. For example, the functiondecode_bytea/function
!function can be used to convert an argument of
!type typebytea/ into unescaped binary.
/para
para
!Similarly, values passed back to productnamePostgreSQL/productname
!must be in the external text representation format. For example, the
!functionencode_bytea/function function can be used
[HACKERS] Feature patch 1 for plperl - open issues
Thanks for all the feedback! I'm going to try to summarize and address the issues raised. (Meanwhile I've started spliting the patch into at least three parts, per Andrew's suggestion: utility functions, the GUCs, the rest.) * Long GUC values I think the underlying assumption that they'll be long values isn't valid. Any non-trivial use should be implemented by loading a private module, like Andrew's example: 'use lib /my/app; use MyApp::Pg;' The lack of multi-line GUCs will naturally encourage that and I'll ensure the docs do as well. * Running arbitrary user-defined code in the postmaster I think there's agreement that the ability to load code in the postmaster is very useful (http://markmail.org/message/wl7dmcbcrkwv2jz2) The concern is only over the safety of combining on_perl_init with shared_preload_libraries. Others have pointed out that the very purpose of shared_preload_libraries is to load and run (init) user-defined code in the postmaster. Albeit in the form of C code. I thought I'd sufficiently addressed the concerns that were raised previously (http://markmail.org/message/u7zhkjkwmztk5hkw). Specifically, on_perl_init is PGC_SUSET (I'll change this to PGC_SIGHUP per Tom's recent email) and SPI functions aren't available for on_*_init code. So the on_*_init code has no natural way to interact with the server. If the code fails (dies/throws an exception) an error is raised. * Running arbitrary user-defined code at backend shutdown In the same way that on_*_init code has no natural way to interact with the server I'm keen for the same to apply to END block code run at backend shutdown. I'd assumed that the SPI functions would fail anyway during shutdown but I hadn't explicitly tested for it (viewing it as a foot shooting gun). I'll explicitly arrange for SPI functions to fail during server shutdown and test for that in the next revision of the patch. What happens when the supplied code has errors takes an unreasonable amount of time to run, does something unsafe, depends on the backend not being in an error state already, etc etc? Since there's no interaction with the server I think the only one of those that needs addressing is takes an unreasonable amount of time to run and (from a different reply) delaying or even preventing shutdown. I'm puzzled by that one as I don't see how it's different to the user doing DO $$ sleep 9 $$ language plperl; # or any other PL I'd be grateful if someone could explain the difference. * Visibility of shared library loading event loading of the plperl shared library should not be a semantically significant event from the user's viewpoint. If these GUCs were PGC_POSTMASTER it wouldn't be so bad, but Tim still seems to want them to be settable inside an individual session before the library is loaded, which makes the loading extremely visible. The perl interpreter has an initial state. The only effect of on_*_init should be to allow the user to choose a different initial state (by running arbitrary code to move from the default state to another). I presume some might say that the first use of a temporary table should not be a semantically significant event from the user's viewpoint. Yet the temp_buffers GUC says: The setting can be changed within individual sessions, but only up until the first use of temporary tables within a session A practical use-case for this plperl.on_*trusted_init behaviour is plperl.on_untrusted_init='use Devel::NYTProf::PgPLPerl' (Or plperl.on_trusted_init if the DBA has pre-loaded it to make it available.) NYTProf needs to be loaded before the code it's going to profile in order to provide full functionality. As an example, if people were using such functionality then the DBA couldn't start preloading plperl for performance without breaking behavior that some of his users might be depending on. This isn't the case. Users can only set the on_trusted_init and on_untrusted_init GUCs. The code in those is only executed when the interpreter is transitioned from its initial HELD state by the user's first use. So the timing of plperl.on_*trusted_init execution isn't altered by shared_preload_libraries. (The on_perl_init GUC, under DBA control, is the only one executed at the time plperl is loaded.) I'd be happy to document plperl.on_trusted_init and on_untrusted_init in the Developer Options section of the docs if that would help set user-expectations. On Sun, Jan 10, 2010 at 07:26:01PM -0500, Tom Lane wrote: Just looking over this patch, I don't think it's nearly robust enough against initialization failures. The original code wasn't very good about that either, but that was (more or less) okay because it was executing predetermined, pretested code that we really don't expect to fail. I think the standard has to be a *lot* higher if we are going to execute user-supplied perl code there. You need to make sure that things are left in a reasonably consistent, safe
Re: [HACKERS] Initial refactoring of plperl.c - updated
On Sun, Jan 10, 2010 at 01:16:13AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Sat, Jan 09, 2010 at 11:16:18PM +0200, Peter Eisentraut wrote: What's the reason for the temp file here? Defensive. If the text2macro.pl program fails/dies then you'd be left with a broken output file with a newer timestamp, so the next make wouldn't rerun text2macro.pl. Doesn't make forcibly remove the target file if the command fails? [ rummages in manual... ] It does if you specify .DELETE_ON_ERROR, which we do (in Makefile.global); so this bit of complication is a waste. Okay. Andrew, perhaps you could apply the attached to fix that. (Or I could bundle it into one of the split out plperl feature patches.) Tim. diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index 43b0fd0..3733da7 100644 *** a/src/pl/plperl/GNUmakefile --- b/src/pl/plperl/GNUmakefile *** include $(top_srcdir)/src/Makefile.shlib *** 57,64 plperl.o: perlchunks.h perlchunks.h: $(PERLCHUNKS) ! $(PERL) $(srcdir)/text2macro.pl --strip='^(\#.*|\s*)$$' $^ perlchunks.htmp ! mv perlchunks.htmp perlchunks.h all: all-lib --- 57,63 plperl.o: perlchunks.h perlchunks.h: $(PERLCHUNKS) ! $(PERL) $(srcdir)/text2macro.pl --strip='^(\#.*|\s*)$$' $^ perlchunks.h all: all-lib *** submake: *** 79,85 $(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X) clean distclean maintainer-clean: clean-lib ! rm -f SPI.c $(OBJS) perlchunks.htmp perlchunks.h rm -rf results rm -f regression.diffs regression.out --- 78,84 $(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X) clean distclean maintainer-clean: clean-lib ! rm -f SPI.c $(OBJS) perlchunks.h rm -rf results rm -f regression.diffs regression.out -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial refactoring of plperl.c - updated
On Fri, Jan 08, 2010 at 09:47:01PM -0500, Andrew Dunstan wrote: Tim Bunce wrote: I see you've not commited it yet, so to help out I've attached a new diff, over the current CVS head, with two minor changes: - Removed the test, as noted above. - Optimized pg_verifymbstr calls to avoid unneeded strlen()s. I have committed this with minor edits. That should give us a clean base for the features patch(es). Wonderful. Many thanks Andrew. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Feature patch 1 for plperl [PATCH]
On Fri, Jan 08, 2010 at 10:36:43PM -0500, Robert Haas wrote: On Fri, Jan 8, 2010 at 10:01 AM, Tim Bunce tim.bu...@pobox.com wrote: I didn't get any significant feedback from the earlier draft so here's the finished 'feature patch 1' for plperl. (This builds on my earlier plperl refactoring patch, and the follow-on ppport.h patch.) Significant changes from the first draft: - New GUC plperl.on_perl_init='...perl...' for admin use. - New GUC plperl.on_trusted_init='...perl...' for plperl user use. - New GUC plperl.on_untrusted_init='...perl...' for plperlu user use. I kind of thought Tom said these were a bad idea, and I think I kind of agree. Tom had some concerns which I believe I've addressed. I'd be happy to hear from Tom if he has any remaining concerns. We're not going to support multi-line values for GUCs AFAIK, so this is going to be pretty kludgy. I'm not sure what you mean by this. Typical use-cases would be: plperl.on_perl_init='use MyStuff;' plperl.on_trusted_init='$some_global=42'; What about making the value a comma-separated list of module names to use, or something? That would force people who just want to set some global variable to write a module. That seems overly painful for no significant gain. The fact that multi-line values for GUCs aren't supported will naturally enourage anyone wanting to execute many statements to write a module for them. That sems like a perfectly reasonable balance. [...] The rest of this all seems pretty nice, though I haven't read the patch yet. Thanks Robert. I look forward to your feedback if you do get a chance to read it. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] mailing list archiver chewing patches
On Sat, Jan 09, 2010 at 02:17:27AM -0300, Alvaro Herrera wrote: Alvaro Herrera wrote: Andrew Dunstan wrote: Tim Bunce's recent patch has been mangled apparently by the list archives. He sent it as an attachment, and that's how I have it in my mailbox, so why isn't it appearing as such in the web archive so that it can be nicely downloaded? See http://archives.postgresql.org/message-id/20100108124613.gl2...@timac.local. It's happened to other people as well: http://archives.postgresql.org/message-id/4b02d3e4.1040...@hut.fi Reviewers and others shouldn't have to cp patches from web pages, especially when it will be horribly line wrapped etc. Can we stop this happening somehow? Try this http://archives.postgresql.org/msgtxt.php?id=20100108124613.gl2...@timac.local That looks like it dumps the raw message. That'll cause problems for any messages using quoted-printable encoding. I'd hazard a guess it also won't do thing right thing for non-charset=us-ascii emails/attachments. This was previously broken for a lot of emails, but I just fixed some of it, and it seems to work for the vast majority of our emails (and certainly for all emails that matter). The other point related to this is that each email should have a link pointing to its text/plain version. This used to be present, but it got broken (I think) at the same time that the anti-email-harvesting measure got broken. I'm going to look at that next. Let me know if you find something broken with this style of link. What's needed is a) a download link for each attachment, regardless of the kind of attachment, and b) the download link should download the content of the attachment in a way that's directly usable. For example, see http://archives.postgresql.org/pgsql-hackers/2010-01/msg00589.php Looking at the raw version of the original message http://archives.postgresql.org/msgtxt.php?id=757953.70187...@web29001.mail.ird.yahoo.com That message has a patch as an attachment: Content-Type: application/octet-stream; name=patch_bit.patch Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename=patch_bit.patch It gets a link in the archive (because it's a non-text content-type I presume): http://archives.postgresql.org/pgsql-hackers/2010-01/bin5ThVOJC3jI.bin but the link doesn't work well. The url ends with .bin and the http response content-type is Content-Type: application/octet-stream so downloaders get a .bin file instead of the original .patch file. It seems that people wanting to send in a patch have two options: send it as text/(something) so it's readable on the archive web page but not copy-n-paste'able because of wordwrapping, or set it as application/octet-stream so it's downloadable but not readable on the web page. Let me know if I've misunderstood anything. Some sugestions: - Provide links for all attachments, whether text/* or not. - For text/* types show the content inline verbatim, don't wrap the text. - If the attachment has a Content-Disposition with a filename then append that to the url. It could simply be a fake 'path info': .../2010-01/bin5ThVOJC3jI.bin/patch_bit.patch - Instead of Description: Binary data on the web page, give the values of the Content-Type and Content-Disposition headers. Tim. p.s. For background... I'm writing an email to the dbi-users dbi-announce mailing lists (~2000 ~5000 users last time I checked) asking anyone who might be interested to help review the plperl feature patch and encouraging them to contribute to the commitfest review process for other patches. It's important that it's *very* easy for these new-comers to follow simple instructions to get involved. I was hoping to be able to use a archives.postgresql.org url to the message with the patch to explain what's the patch does _and_ provide a download link. It seems I'll have to upload the patch somewhere else. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial refactoring of plperl.c - updated
On Sat, Jan 09, 2010 at 11:16:18PM +0200, Peter Eisentraut wrote: On fre, 2010-01-08 at 12:46 +, Tim Bunce wrote: *** 45,50 --- 45,55 include $(top_srcdir)/src/Makefile.shlib + plperl.o: perlchunks.h + + perlchunks.h: plc_*.pl + $(PERL) text2macro.pl --strip='^(\#.*|\s*)$$' plc_*.pl perlchunks.htmp + mv perlchunks.htmp perlchunks.h all: all-lib What's the reason for the temp file here? Defensive. If the text2macro.pl program fails/dies then you'd be left with a broken output file with a newer timestamp, so the next make wouldn't rerun text2macro.pl. Tim. p.s. In the makefile for perl we use a little utility called mv_if_diff instead of a plain mv so that any downstream dependencies only get rebuilt if the contents have changed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial refactoring of plperl.c - updated
On Sat, Jan 09, 2010 at 11:49:22PM +, Tim Bunce wrote: On Sat, Jan 09, 2010 at 11:16:18PM +0200, Peter Eisentraut wrote: On fre, 2010-01-08 at 12:46 +, Tim Bunce wrote: *** 45,50 --- 45,55 include $(top_srcdir)/src/Makefile.shlib + plperl.o: perlchunks.h + + perlchunks.h: plc_*.pl + $(PERL) text2macro.pl --strip='^(\#.*|\s*)$$' plc_*.pl perlchunks.htmp + mv perlchunks.htmp perlchunks.h all: all-lib What's the reason for the temp file here? Defensive. If the text2macro.pl program fails/dies then you'd be left with a broken output file with a newer timestamp, so the next make wouldn't rerun text2macro.pl. An alternative would be for text2macro.pl to write to a temp file and rename at the end. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Initial refactoring of plperl.c - updated
On Mon, Jan 04, 2010 at 06:38:03PM -0500, Andrew Dunstan wrote:
Andrew Dunstan wrote:
Yes. I believe the test is highlighting an existing problem: that plperl
function in non-PG_UTF8 databases can't use regular expressions that
require unicode character meta-data.
Either the (GetDatabaseEncoding() == PG_UTF8) test in plperl_safe_init()
should be removed, so the utf8fix function is always called, or the
test should be removed (or hacked to only apply to PG_UTF8 databases).
I tried forcing the test, but it doesn't seem to work, possibly
because in the case that the db is not utf8 we aren't forcing
argument strings to UTF8 :-(
I think we might need to remove the test from the patch.
I have not been able to come up with a fix for this - the whole
thing seems very fragile. I'm going to commit what remains of this
patch, but not add the extra regression test. I'll add a TODO to
allow plperl to do utf8 operations in non-utf8 databases.
I see you've not commited it yet, so to help out I've attached
a new diff, over the current CVS head, with two minor changes:
- Removed the test, as noted above.
- Optimized pg_verifymbstr calls to avoid unneeded strlen()s.
This should apply cleanly to cvs, saving you the need to resolve the
conflicts caused by the recent pg_verifymbstr patch.
I'll add it to the commitfest once it reaches the archives.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 7eebfba..37114bd 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
***
*** 14,20
para
PL/Perl is a loadable procedural language that enables you to write
productnamePostgreSQL/productname functions in the
!ulink url=http://www.perl.com;Perl programming language/ulink.
/para
para
--- 14,20
para
PL/Perl is a loadable procedural language that enables you to write
productnamePostgreSQL/productname functions in the
!ulink url=http://www.perl.org;Perl programming language/ulink.
/para
para
*** SELECT * FROM perl_set();
*** 313,319
use strict;
/programlisting
in the function body. But this only works in applicationPL/PerlU/
!functions, since literaluse/ is not a trusted operation. In
applicationPL/Perl/ functions you can instead do:
programlisting
BEGIN { strict-import(); }
--- 313,320
use strict;
/programlisting
in the function body. But this only works in applicationPL/PerlU/
!functions, since the literaluse/ triggers a literalrequire/
!which is not a trusted operation. In
applicationPL/Perl/ functions you can instead do:
programlisting
BEGIN { strict-import(); }
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..8989b14 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*** PSQLDIR = $(bindir)
*** 45,50
--- 45,55
include $(top_srcdir)/src/Makefile.shlib
+ plperl.o: perlchunks.h
+
+ perlchunks.h: plc_*.pl
+ $(PERL) text2macro.pl --strip='^(\#.*|\s*)$$' plc_*.pl perlchunks.htmp
+ mv perlchunks.htmp perlchunks.h
all: all-lib
*** submake:
*** 65,71
$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
clean distclean maintainer-clean: clean-lib
! rm -f SPI.c $(OBJS)
rm -rf results
rm -f regression.diffs regression.out
--- 70,76
$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
clean distclean maintainer-clean: clean-lib
! rm -f SPI.c $(OBJS) perlchunks.htmp perlchunks.h
rm -rf results
rm -f regression.diffs regression.out
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index ...d2d5518 .
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 0
--- 1,50
+ SPI::bootstrap();
+ use vars qw(%_SHARED);
+
+ sub ::plperl_warn {
+ (my $msg = shift) =~ s/\(eval \d+\) //g;
+ elog(NOTICE, $msg);
+ }
+ $SIG{__WARN__} = \::plperl_warn;
+
+ sub ::plperl_die {
+ (my $msg = shift) =~ s/\(eval \d+\) //g;
+ die $msg;
+ }
+ $SIG{__DIE__} = \::plperl_die;
+
+ sub ::mkunsafefunc {
+ my $ret = eval(qq[ sub { $_[0] $_[1] } ]);
+ $@ =~ s/\(eval \d+\) //g if $@;
+ return $ret;
+ }
+
+ use strict;
+
+ sub ::mk_strict_unsafefunc {
+ my $ret = eval(qq[ sub { use strict; $_[0] $_[1] } ]);
+ $@ =~ s/\(eval \d+\) //g if $@;
+ return $ret;
+ }
+
+ sub ::_plperl_to_pg_array {
+ my $arg = shift;
+ ref $arg eq 'ARRAY' || return $arg;
+ my $res = '';
+ my $first = 1;
+ foreach my $elem (@$arg) {
+ $res .= ', ' unless $first; $first = undef;
+ if (ref $elem) {
+ $res .= _plperl_to_pg_array($elem);
+ }
+ elsif (defined($elem)) {
+ my $str = qq($elem);
+ $str =~ s/([\\\])/\\$1/g;
+ $res .= qq(\$str\);
+ }
+ else {
+ $res .= 'NULL' ;
+ }
+ }
+ return qq({$res});
+ }
diff --git a/src/pl/plperl/plc_safe_bad.pl
Re: [HACKERS] Testing plperl-plperlu interaction
On Wed, Jan 06, 2010 at 07:07:12PM -0500, Andrew Dunstan wrote: Alvaro Herrera wrote: decibel wrote: We've actually run into similar issues. Alvaro came up with a patch that fixes our specific issue, but I think he said there were some other cases that needed to be fixed as well. Anyone looking to fix this should ping Alvaro first. No, what I fixed was the contrib/xml2 crasher that I still haven't submitted here (sorry). The plperl fix came from Alexey Klyukin, and AFAIK the fix is in 8.4.2. Yes. [thinks] Could we have a regression test for it with an alternative result file for the case where plperl and plperlu are not allowed to run together? Or perhaps put the tests that require multiplicity into a plperl_multi.sql file and add logic to the GNUmakefile to add that file to REGRESS if perl -V:usemultiplicity returns usemultiplicity='define'; Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of plperl inter-sp calling
On Wed, Jan 06, 2010 at 08:46:11PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Wed, Jan 06, 2010 at 01:45:45PM -0800, David E. Wheeler wrote: On Jan 6, 2010, at 12:20 PM, Tom Lane wrote: One of the things on my to-do list for today is to make configure reject Perl versions less than $TBD. I thought we had agreed a week or so back that 5.8 was the lowest safe version because of utf8 and other considerations. +1, and 5.8.3 at a minimum for utf8 stuff, 5.8.8 much much better. I think we said 5.8.1 at the time, but 5.8.3 sounds good to me. There would be _very_ few places using 5.8.6. I went with 5.8 as the cutoff, for a couple of reasons: we're not in the business of telling people they ought to be up-to-date, but only of rejecting versions that demonstrably fail badly; I think 5.8.0 will fail badly, possibly demonstrably but more likely in subtle ways relating to utf8 that are hard to debug. and I found out that older versions of awk are not sufficiently competent with and || to code a more complex test properly :-(. A version check that doesn't actually do what it claims to is worse than useless, and old buggy awk is exactly what you'd expect to find on a box with old buggy perl. Either of these approaches should work back to perl 5.0... perl -we 'use 5.008001' 2/dev/null echo ok or perl -we 'exit($] 5.008001)' echo ok (It's also worth noting that the perl version seen at configure time is not necessarily that seen at runtime, anyway, so there's not a lot of point in getting too finicky here.) A simple use 5.008001; at the start of src/pl/plperl/plc_perlboot.pl would address that. I believe Andrew is planing to commit my plperl refactor patch soon. He could add it then, or I could add it to my feature patch (which I plan to reissue soon, with very minor changes, and add to commitfest). Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Add .gitignore files to CVS?
I've a .git/info/exclude file I pulled from a link on the dev wiki. Some of the changes I'm making create new files that ought to be added to the excluded files. I can easily add them to my .git/info/exclude file but it's much more work for me and others to spread those changes. Is there any reason not to add .gitignore files into the repository? They'll make no difference to those who don't use git, but be very helpful to, and maintained by, those who do. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add .gitignore files to CVS?
On Thu, Jan 07, 2010 at 05:45:08PM -0500, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On tor, 2010-01-07 at 22:16 +, Tim Bunce wrote: Is there any reason not to add .gitignore files into the repository? I already find the .cvsignore files to be useless and an annoyance to keep up to date (well, I basically ignore them and someone else cleans up after me), When/if we move off CVS, we should drop the .cvsignore files and insert .gitignore (assuming that works the same way). I am not in favor of expecting the core project to maintain support for non-core SCMs though. Will we have .bzrignore and who knows what else in there too? As Peter points out, the only way the things will get maintained is if the complaints they suppress are in-the-face of somebody with commit access to the core repo. I like quiet from my cvs updates, so I'm willing to maintain .cvsignores, but I'm not willing to maintain multiple copies of that information. How about a make gitignore target that copies each .cvsignore file to a .gitignore file and appends .gitignore to it? That way git users can just do make gitignore to get working .gitignore files without cluttering the repository. As a bonus they'll then have an incentive to update the .cvsignore files. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of plperl inter-sp calling
On Tue, Jan 05, 2010 at 06:54:36PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Thu, Dec 31, 2009 at 09:47:24AM -0800, David E. Wheeler wrote: Definite benefit, there. How does it handle the difference between IMMUTABLE | STABLE | VOLATILE, as well as STRICT functions? It doesn't at the moment. I think IMMUTABLE, STABLE and VOLATILE can be (documented as being) ignored in this context. Just for the record, I think that would be a seriously bad idea. There is a semantic difference there (having to do with snapshot management), and ignoring it would mean that a function could behave subtly differently depending on how it was called. It's the kind of thing that would be a nightmare to debug, too, because you'd never see a problem except when the right sort of race condition occurred with another transaction. I see downthread that you seem to have an approach without this gotcha, so that's fine, but I wanted to make it clear that you can't just ignore volatility. Ok, thanks Tom. For my own benefit, being a PostgreSQL novice, could you expand a little? For example, given two stored procedures, A and V, where V is marked VOLATILE and both are plperl. How would having A call V directly, within the plperl interpreter, cause problems? Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of plperl inter-sp calling
On Tue, Jan 05, 2010 at 07:06:35PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: The only question I have at the moment, before I try implementing this, is the the need for freeing the plan. When would that be needed? There's probably no strong need to do it at all, That's good. unless you are dropping your last reference to the plan. Uh, now I'm confused again. The way I envisage it, each imported function would contain a plan. So each would have the one and only reference to that plan. So, if there was a need to drop them, I would be dropping the last reference to the plan. Let me ask the question another way... is there any reason to drop plans other than limiting memory usage? I couldn't find anything in the docs to suggest there was but want to be sure. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of plperl inter-sp calling
On Wed, Jan 06, 2010 at 01:45:45PM -0800, David E. Wheeler wrote: On Jan 6, 2010, at 12:20 PM, Tom Lane wrote: One of the things on my to-do list for today is to make configure reject Perl versions less than $TBD. I thought we had agreed a week or so back that 5.8 was the lowest safe version because of utf8 and other considerations. +1, and 5.8.3 at a minimum for utf8 stuff, 5.8.8 much much better. I think we said 5.8.1 at the time, but 5.8.3 sounds good to me. There would be _very_ few places using 5.8.6. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of plperl inter-sp calling
On Wed, Jan 06, 2010 at 11:41:46AM -0500, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Next question: what do we do if a direct-called function calls return_next()? That at least must surely take effect in the caller's context - the callee won't have anywhere to stash the the results at all. Whatever do you mean by take effect in the caller's context? I surely hope it's not return the row to the caller's caller, who likely isn't expecting anything of the kind. I suspect Tim will just answer that he isn't going to try to short-circuit the call path for set-returning functions. For 8.5 I don't think I'll even attempt direct inter-plperl-calls. I'll just do a nicely-sugared wrapper around spi_exec_prepared(). Either via import, as I outlined earlier, or Garick Hamlin's suggestion of attributes - which is certainly worth exploring. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
