Re: Feature patch 1 for plperl [PATCH]

On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> writes:
>> On Fri, Jan 08, 2010 at 10:36:43PM -0500, Robert Haas wrote:
>>> 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.
>
> You haven't addressed them, you've simply ignored them.

That's not *completely* true.

http://archives.postgresql.org/pgsql-hackers/2009-12/msg00432.php

> For the record,
> I think it's a bad idea to run arbitrary user-defined code in the
> postmaster, and I think it's a worse idea to run arbitrary user-defined
> code at backend shutdown (the END-blocks bit).  I do not care in the
> least what applications you think this might enable --- the negative
> consequences for overall system stability seem to me to outweigh any
> possible arguments on that side.  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?

Same thing that happens when you put something stupid into
shared_preload_libraries - you destabilize your database cluster and
the world blows up.

> I do not have a veto over stuff like this, but if I did, it would
> not go in.

I'm not as strongly opposed to this as you are, but I definitely think
there will be some people who shoot themselves in the foot with it. I
don't think it's necessarily more dangerous than
shared_preload_libraries from a theoretical standpoint, but the sheer
fact that this is Perl rather than C means more people will try to do
it, and some of them will manage to take out the whole database
cluster, which will not be awesome.

I think this is a real weakness of our one-process-per-connection
model. If it were possible to recycle backends for new connections,
there would be no need even to consider doing things like this. Yeah,
I know you don't want to do that either, just mentioning it...

>>> 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".
>
> What he means by "this" is defining GUCs in a way that would make people
> want to use multi-line values for them.  However, that doesn't have
> anything to do with my worries ...

Well, you did mention it previously.

http://archives.postgresql.org/pgsql-hackers/2009-12/msg00384.php

Anyway, I think you've put this pretty well here: the current
definition will make people WANT to use multi-line values for this,
and we don't support that. I think Tim's example is fairly contrived
- setting a global variable here does not seem likely to be useful to
very many users, and the ones who do want it will likely want also
want multi-line values. What IS likely to be useful is preloading a
bunch of perl modules so that backend startup doesn't take an
unreasonably long time. It's nicer to write:

plperl.on_perl_init='strict,warnings,LDAP,HTML::Parser,Archive::Zip'

rather than:

plperl.on_perl_init='use strict;use warnings;use LDAP;use
HTML::Parser;use Archive::Zip;'

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.

...Robert