[PATCHES] PL/Perl namespace fix
The attached patch appears to fix the problem discussed in the "plperl gives error after reconnect" thread in pgsql-bugs: http://archives.postgresql.org/pgsql-bugs/2005-08/msg00133.php Unfortunately I don't understand *why* it fixes the problem, which means I'm not sure that it's the correct fix. As Tom Lane and I discovered, Perl builds with threading don't appear to have a problem, while Perl builds without threading do. It's not clear to me why "mksafefunc" works in some Perl installations but "::mksafefunc" is required in others, given otherwise identical test conditions. Can anybody explain? If this is indeed the correct fix then it should probably be backpatched to 8.0. -- Michael Fuhr Index: plperl.c === RCS file: /projects/cvsroot/pgsql/src/pl/plperl/plperl.c,v retrieving revision 1.89 diff -c -r1.89 plperl.c *** plperl.c12 Aug 2005 21:26:32 - 1.89 --- plperl.c20 Aug 2005 18:35:14 - *** *** 680,686 * errors properly. Perhaps it's because there's another level of * eval inside mksafefunc? */ ! count = perl_call_pv((trusted ? "mksafefunc" : "mkunsafefunc"), G_SCALAR | G_EVAL | G_KEEPERR); SPAGAIN; --- 680,686 * errors properly. Perhaps it's because there's another level of * eval inside mksafefunc? */ ! count = perl_call_pv((trusted ? "::mksafefunc" : "::mkunsafefunc"), G_SCALAR | G_EVAL | G_KEEPERR); SPAGAIN; ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] PL/Perl namespace fix
Michael Fuhr <[EMAIL PROTECTED]> writes: > The attached patch appears to fix the problem discussed in the > "plperl gives error after reconnect" thread in pgsql-bugs: I think this patch is a good idea just on grounds of namespace safety, so I'll go ahead and apply it. But like you, I have no idea why it appears to fix the reported problem --- there don't seem to be any namespace hazards in the test case. I think there is still a problem here somewhere. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] PL/Perl namespace fix
Tom Lane wrote: Michael Fuhr <[EMAIL PROTECTED]> writes: The attached patch appears to fix the problem discussed in the "plperl gives error after reconnect" thread in pgsql-bugs: I think this patch is a good idea just on grounds of namespace safety, so I'll go ahead and apply it. But like you, I have no idea why it appears to fix the reported problem --- there don't seem to be any namespace hazards in the test case. I think there is still a problem here somewhere. Yes, very puzzling, but as you say the fix seems correct and harmless. How far back has this bug been verified? Was it in 7.4 before we made some large changes to plperl? cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] PL/Perl embedding string common elements
The attached patch moves the common elements of loose_embedding[]
and strict_embedding[] to a macro so they can be maintained in
one place. As Tom Lane noticed, ::_plperl_to_pg_array was missing
from strict_embedding[], which appears to be a bug.
http://archives.postgresql.org/pgsql-bugs/2005-08/msg00189.php
I'm not sure this patch meets Tom's request to avoid "too much
violence to the readability," mostly because of the added backslashes
at the end of the macro's lines. Opinions?
--
Michael Fuhr
Index: plperl.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plperl/plperl.c,v
retrieving revision 1.89
diff -c -r1.89 plperl.c
*** plperl.c12 Aug 2005 21:26:32 - 1.89
--- plperl.c20 Aug 2005 19:23:19 -
***
*** 189,230
static void
plperl_init_interp(void)
{
static char*loose_embedding[3] = {
"", "-e",
/* all one string follows (no commas please) */
! "SPI::bootstrap(); use vars qw(%_SHARED);"
! "sub ::plperl_warn { my $msg = shift; &elog(&NOTICE, $msg); } "
! "$SIG{__WARN__} = \\&::plperl_warn; "
"sub ::mkunsafefunc {return eval(qq[ sub { $_[0] $_[1] } ]); }"
- "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); "
- "} "
- "else "
- "{ "
- " my $str = qq($elem); "
- " $str =~ s/([\"])/$1/g; "
- " $res .= qq(\"$str\"); "
- "} "
- " } "
- " return qq({$res}); "
- "} "
};
-
static char*strict_embedding[3] = {
"", "-e",
/* all one string follows (no commas please) */
! "SPI::bootstrap(); use vars qw(%_SHARED);"
! "sub ::plperl_warn { my $msg = shift; &elog(&NOTICE, $msg); } "
! "$SIG{__WARN__} = \\&::plperl_warn; "
"sub ::mkunsafefunc {return eval("
"qq[ sub { use strict; $_[0] $_[1] } ]); }"
};
--- 189,230
static void
plperl_init_interp(void)
{
+ #define COMMON_EMBEDDING \
+ "SPI::bootstrap(); use vars qw(%_SHARED);" \
+ "sub ::plperl_warn { my $msg = shift; &elog(&NOTICE, $msg); } "
\
+ "$SIG{__WARN__} = \\&::plperl_warn; " \
+ "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); " \
+ "} " \
+ "else " \
+ "{ " \
+ " my $str = qq($elem); " \
+ " $str =~ s/([\"])/$1/g; " \
+ " $res .= qq(\"$str\"); " \
+ "} " \
+ " } " \
+ " return qq({$res}); " \
+ "} "
+
static char*loose_embedding[3] = {
"", "-e",
/* all one string follows (no commas please) */
! COMMON_EMBEDDING
"sub ::mkunsafefunc {return eval(qq[ sub { $_[0] $_[1] } ]); }"
};
static char*strict_embedding[3] = {
"", "-e",
/* all one string follows (no commas please) */
! COMMON_EMBEDDING
"sub ::mkunsafefunc {return eval("
"qq[ sub { use strict; $_[0] $_[1] } ]); }"
};
---(end of broadcast)---
TIP 6: explain analyze is your friend
[PATCHES] PL/Perl regression tests with use_strict
The attached patch allows the PL/Perl regression tests to pass when
use_strict is enabled. I've also attached a variant of plperl_elog.out
to account for an elog() message that shows a different line number
when run under use_strict.
--
Michael Fuhr
Index: src/pl/plperl/sql/plperl.sql
===
RCS file: /projects/cvsroot/pgsql/src/pl/plperl/sql/plperl.sql,v
retrieving revision 1.4
diff -c -r1.4 plperl.sql
*** src/pl/plperl/sql/plperl.sql12 Jul 2005 01:16:22 - 1.4
--- src/pl/plperl/sql/plperl.sql20 Aug 2005 19:38:39 -
***
*** 240,246
--
CREATE OR REPLACE FUNCTION perl_srf_rn() RETURNS SETOF RECORD AS $$
! $i = 0;
for ("World", "PostgreSQL", "PL/Perl") {
return_next({f1=>++$i, f2=>'Hello', f3=>$_});
}
--- 240,246
--
CREATE OR REPLACE FUNCTION perl_srf_rn() RETURNS SETOF RECORD AS $$
! my $i = 0;
for ("World", "PostgreSQL", "PL/Perl") {
return_next({f1=>++$i, f2=>'Hello', f3=>$_});
}
***
*** 253,260
--
CREATE OR REPLACE FUNCTION perl_spi_func() RETURNS SETOF INTEGER AS $$
! $x = spi_query("select 1 as a union select 2 as a");
! while (defined ($y = spi_fetchrow($x))) {
return_next($y->{a});
}
return;
--- 253,260
--
CREATE OR REPLACE FUNCTION perl_spi_func() RETURNS SETOF INTEGER AS $$
! my $x = spi_query("select 1 as a union select 2 as a");
! while (defined (my $y = spi_fetchrow($x))) {
return_next($y->{a});
}
return;
Index: src/pl/plperl/expected/plperl.out
===
RCS file: /projects/cvsroot/pgsql/src/pl/plperl/expected/plperl.out,v
retrieving revision 1.4
diff -c -r1.4 plperl.out
*** src/pl/plperl/expected/plperl.out 12 Jul 2005 01:16:22 - 1.4
--- src/pl/plperl/expected/plperl.out 20 Aug 2005 19:38:39 -
***
*** 336,342
-- Test return_next
--
CREATE OR REPLACE FUNCTION perl_srf_rn() RETURNS SETOF RECORD AS $$
! $i = 0;
for ("World", "PostgreSQL", "PL/Perl") {
return_next({f1=>++$i, f2=>'Hello', f3=>$_});
}
--- 336,342
-- Test return_next
--
CREATE OR REPLACE FUNCTION perl_srf_rn() RETURNS SETOF RECORD AS $$
! my $i = 0;
for ("World", "PostgreSQL", "PL/Perl") {
return_next({f1=>++$i, f2=>'Hello', f3=>$_});
}
***
*** 354,361
-- Test spi_query/spi_fetchrow
--
CREATE OR REPLACE FUNCTION perl_spi_func() RETURNS SETOF INTEGER AS $$
! $x = spi_query("select 1 as a union select 2 as a");
! while (defined ($y = spi_fetchrow($x))) {
return_next($y->{a});
}
return;
--- 354,361
-- Test spi_query/spi_fetchrow
--
CREATE OR REPLACE FUNCTION perl_spi_func() RETURNS SETOF INTEGER AS $$
! my $x = spi_query("select 1 as a union select 2 as a");
! while (defined (my $y = spi_fetchrow($x))) {
return_next($y->{a});
}
return;
-- test warnings and errors from plperl
create or replace function perl_elog(text) returns void language plperl as $$
my $msg = shift;
elog(NOTICE,$msg);
$$;
select perl_elog('explicit elog');
NOTICE: explicit elog
perl_elog
---
(1 row)
create or replace function perl_warn(text) returns void language plperl as $$
my $msg = shift;
warn($msg);
$$;
select perl_warn('implicit elog via warn');
NOTICE: implicit elog via warn at (eval 9) line 4.
perl_warn
---
(1 row)
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [PATCHES] PL/Python error checking
On Mon, Jul 11, 2005 at 08:13:24PM -0600, Michael Fuhr wrote: > On Sun, Jul 10, 2005 at 12:58:24AM -0400, Bruce Momjian wrote: > > I am unclear about backpatching this. We have to weigh the risks of > > applying or not applying to 8.0.X. Comments? > > Since 7.4, PL/Python is only available as an untrusted language, > so only a database superuser could create an exploitable function. > However, it might be possible for an ordinary user to tickle the > bug by calling such a function and passing it certain data, either > as an argument or as table data. The code is buggy in any case: > PyObject_Str() is documented to return NULL on error, and > PyString_AsString() doesn't expect a NULL pointer so it segfaults > if passed one. Since the patch simply checks for that condition > and raises an error instead of calling a function that will segfault > and take down the backend, I can't think of what risk applying the > patch would have. The greater risk would seem to be in not applying > it. I haven't seen this patch applied to other than HEAD. Since it fixes a segmentation fault, should it be backpatched before the new releases? Here's the original patch submission: http://archives.postgresql.org/pgsql-patches/2005-06/msg00519.php -- Michael Fuhr ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] PL/Perl namespace fix
On Sat, Aug 20, 2005 at 03:30:07PM -0400, Andrew Dunstan wrote: > How far back has this bug been verified? Was it in 7.4 before we made > some large changes to plperl? Not sure -- the problem showed up with SPI, which wasn't in the stock PL/Perl until 8.0. I haven't done tests with earlier versions, which would presumably require DBD::PgSPI. -- Michael Fuhr ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] prevent encoding conversion recursive error
Peter Eisentraut wrote: > Am Sonntag, 14. August 2005 23:48 schrieb Tom Lane: > > Yeah, but don't we already have some code for that (or, actually, the > > reverse direction) in initdb? It's probably not perfect, but it'd be > > a lot better than crashing. > > The reverse direction is a lot simpler because we know the set of possible > output values. I'm not sure how to do the mapping in the direction of the > OS. Is there a TODO here? -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] A couple of patches for PostgreSQL 64bit support
Koichi Suzuki <[EMAIL PROTECTED]> writes: > Here're a couple of patches for PostgreSQL 64bit support. There're just > two extension to 64bit, size of shared memory and transaction ID. I've applied the part of this that seemed reasonably noncontroversial, namely the fixes to do shared memory size calculation in size_t arithmetic instead of int arithmetic. (While at it, I extended the Size convention to all the shared memory sizing routines, not just buffers, and added code to detect overflows in the calculations. That way we don't need a "64 bit" configure switch.) While I still remain unconvinced that there's any real-world need for more than 2Gb of shared_buffers, this change certainly makes the code more robust against configuration errors, and it has essentially zero cost (except maybe a few more milliseconds during postmaster start). On the other hand, I think the 64-bit XID idea needs considerably more work before being proposed again. That would incur serious costs due to the expansion of tuple headers, and there's no evidence that the distributed cost would be bought back by avoiding one vacuum pass every billion transactions. (Your description of the patch claimed that vacuuming requires shutting down the database, which is simply wrong.) Also, as previously noted, you can't just whack the size of XID around without considering side-effects on CID, OID, Datum, etc. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] prevent encoding conversion recursive error
Bruce Momjian writes: > Is there a TODO here? Yeah: * Fix problems with wrong runtime encoding conversion for NLS message files One thing that occurred to me is that we might be able to simplify the problem by adopting a project standard that all NLS message files shall be in UTF8, period. Then we only have one encoding name to figure out rather than N. Maybe this doesn't help much ... regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] PL/Perl embedding string common elements
Michael Fuhr <[EMAIL PROTECTED]> writes: > The attached patch moves the common elements of loose_embedding[] > and strict_embedding[] to a macro so they can be maintained in > one place. As Tom Lane noticed, ::_plperl_to_pg_array was missing > from strict_embedding[], which appears to be a bug. Actually, it strikes me that this is working around a problem that we shouldn't have in the first place: there should not be two different embedding strings. I hadn't noticed the use_strict GUC before, but now that I see it, I think it's implemented in a completely awful way. Because the embedding string is only executed once per backend, it's effectively impossible to change use_strict on the fly. I think we should be looking for another implementation of that feature, rather than just cleaning up the minor breakage. One would expect changes in the variable to at least affect future function compilations in the current backend, if not indeed recompiling already-compiled ones. I'm not much of a Perl guru, but could we make this happen by having four functions instead of two? (mksafefunc, mksafestrictfunc, etc) I'm not too clear on what the scope of effects of 'use strict' is. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] PL/Perl embedding string common elements
Tom Lane wrote: Michael Fuhr <[EMAIL PROTECTED]> writes: The attached patch moves the common elements of loose_embedding[] and strict_embedding[] to a macro so they can be maintained in one place. As Tom Lane noticed, ::_plperl_to_pg_array was missing from strict_embedding[], which appears to be a bug. Actually, it strikes me that this is working around a problem that we shouldn't have in the first place: there should not be two different embedding strings. I hadn't noticed the use_strict GUC before, but now that I see it, I think it's implemented in a completely awful way. Because the embedding string is only executed once per backend, it's effectively impossible to change use_strict on the fly. I think we should be looking for another implementation of that feature, rather than just cleaning up the minor breakage. One would expect changes in the variable to at least affect future function compilations in the current backend, if not indeed recompiling already-compiled ones. I'm not much of a Perl guru, but could we make this happen by having four functions instead of two? (mksafefunc, mksafestrictfunc, etc) I'm not too clear on what the scope of effects of 'use strict' is. The intention was to make it settable on startup only. Unfortunately when I tried that it blew up on me. It seems that the getting of custom settings happens too late for that to work. I recall posting a message to -hackers asking for some help on that, but got no response, so I just made it PGC_USERSET so I could make some progress. The scope of "use strict" is from the declaration down to the end of the enclosing block, file, or eval. The biggest problem is that "use" is in fact a forbidden operation in trusted plperl. If you wanted a simpler (and in a way more perlish) way of enabling this, we could abandon the GUC var altogether and load the strict module unconditionally. Then each user function could turn on strict mode at their choice by calling "strict->import();", possibly in a BEGIN block (plperlu functions could just have "use strict;"). Every perl module whose author wants strict mode (and they all should) has to carry such a declaration, so in a sense we'd just be doing what perl itself does, and by trying to provide a global switch we're being unperlish. Of course, many people regret that strict mode was not (mainly so that millions of lines of legacy code didn't break) made the default in perl5. :-) I know of one ardent plperl advocate at least who would be highly disappointed if we followed this course. We'd still have made an advance, because in 8.0 you can't turn on strict mode at all in trusted plperl. Of course, we're getting close to Beta, but if you're happy redesigning things at this stage, we should be able to wrap it up fairly quickly - it's hardly huge. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] PL/Perl embedding string common elements
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Every perl module whose author wants strict mode (and they all should) > has to carry such a declaration, so in a sense we'd just be doing what > perl itself does, and by trying to provide a global switch we're being > unperlish. You missed my point. I wasn't objecting to having the global switch, only to the fact that turning it on and off doesn't do what a rational person would expect. If it's going to advertise itself as USERSET then it darn well ought to be settable. The idea of loading the strict module unconditionally seems ok to me, if we can work out a way of making it apply or not apply to individual function compilations. From what you were saying, perhaps it would work to implicitly add "strict->import();" when use_strict is enabled? regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] PL/Perl embedding string common elements
Tom Lane wrote:
Andrew Dunstan <[EMAIL PROTECTED]> writes:
Every perl module whose author wants strict mode (and they all should)
has to carry such a declaration, so in a sense we'd just be doing what
perl itself does, and by trying to provide a global switch we're being
unperlish.
You missed my point. I wasn't objecting to having the global switch,
only to the fact that turning it on and off doesn't do what a rational
person would expect. If it's going to advertise itself as USERSET then
it darn well ought to be settable.
The idea of loading the strict module unconditionally seems ok to me,
if we can work out a way of making it apply or not apply to individual
function compilations. From what you were saying, perhaps it would
work to implicitly add "strict->import();" when use_strict is enabled?
I am pretty sure we could load it unconditionally without disturbing
anything (because of the scope rules), and then import it appropriately.
Do you expect turning it on to affect only future compilations? Or
should we recompile every function already compiled in the present
backend? I can see arguments either way. What about if we turn it off?
In theory, anything that works with it turned on should work fine with
it turned off, although the reverse is not true of course ...
And yes, you're right, we could have 4 functions instead of two:
{safe,unsafe} x {strict, unstrict}.
cheers
andrew
---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [PATCHES] PL/Perl embedding string common elements
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Do you expect turning it on to affect only future compilations? Or > should we recompile every function already compiled in the present > backend? I can see arguments either way. Yeah, me too. I would definitely expect a change in the variable (in either direction) to be respected in subsequent function compilations. I'm less excited about redoing previous compilations; a case could be made for doing so, but I won't insist on it. I think the main case where use_strict is interesting is in development, where you're repeatedly CREATE OR REPLACE'ing the function and retesting it, so you're going to be doing new compilations anyway. Looking ahead to the future a bit, is there any reason to expect that use_strict would have cross-function effects? In a normal Perl script I suppose it does add some cross-function checking. Right now, with plperl functions all mutually anonymous, there are no interactions to be strict about --- but it says in the todo list that that's something to fix someday. When that happens, would it actually be correct or even essential to force use_strict to have just one value throughout a backend's lifespan? If so, the existing code isn't so unreasonable, but we need to fix the docs ... regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
