Thread

1. Feature patch 1 for plperl [PATCH]

   Tim Bunce <tim.bunce@pobox.com> — 2010-01-08T15:01:07Z

   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.
   - END blocks now run at backend exit (fixes bug #5066).
   - Stored procedure subs are now given names ($name__$oid).
   - More error checking and reporting.
   - Warnings no longer have an extra newline in the NOTICE text.
   - Various minor optimizations like pre-growing data structures.
   
   Additional changes from the second draft:
   - SPI functions aren't available during plperl.on_*_init execution.
   - Added utility functions: quote_literal, quote_nullable, quote_ident,
       encode_bytea, decode_bytea, looks_like_number,
       encode_array_literal, encode_array_constructor.
   - Enabled plperl to "use"/"require" safely by redirecting the require
       opcode to code that dies if module not already loaded.
   - Corresponding changes to the documentation.
   
   Additional changes in this version:
   - Added the missing ', arguments' to docs of spi_exec_prepared().
   - Added Util.c to list of files for plperl make clean to delete.
   
   I'll add this to the commitfest.
   
   Tim.
   
   

2. Re: Feature patch 1 for plperl [PATCH]

   David E. Wheeler <david@kineticode.com> — 2010-01-08T18:32:36Z

   On Jan 8, 2010, at 7:01 AM, Tim Bunce 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.
   > - END blocks now run at backend exit (fixes bug #5066).
   > - Stored procedure subs are now given names ($name__$oid).
   > - More error checking and reporting.
   > - Warnings no longer have an extra newline in the NOTICE text.
   > - Various minor optimizations like pre-growing data structures.
   > 
   > Additional changes from the second draft:
   > - SPI functions aren't available during plperl.on_*_init execution.
   > - Added utility functions: quote_literal, quote_nullable, quote_ident,
   >    encode_bytea, decode_bytea, looks_like_number,
   >    encode_array_literal, encode_array_constructor.
   > - Enabled plperl to "use"/"require" safely by redirecting the require
   >    opcode to code that dies if module not already loaded.
   > - Corresponding changes to the documentation.
   > 
   > Additional changes in this version:
   > - Added the missing ', arguments' to docs of spi_exec_prepared().
   > - Added Util.c to list of files for plperl make clean to delete.
   > 
   > I'll add this to the commitfest.
   
   These changes all sound great to me, Tim, and if I can ever get my PL/Perl install working again, I'd be glad to find some tuits and review it.
   
   Best,
   
   David
   
   
   

3. Re: Feature patch 1 for plperl [PATCH]

   Robert Haas <robertmhaas@gmail.com> — 2010-01-09T03:36:43Z

   On Fri, Jan 8, 2010 at 10:01 AM, Tim Bunce <Tim.Bunce@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.ef
   > - 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.  We're not going to support multi-line values for GUCs
   AFAIK, so this is going to be pretty kludgy.  What about making the
   value a comma-separated list of module names to use, or something?
   
   > - END blocks now run at backend exit (fixes bug #5066).
   > - Stored procedure subs are now given names ($name__$oid).
   > - More error checking and reporting.
   > - Warnings no longer have an extra newline in the NOTICE text.
   > - Various minor optimizations like pre-growing data structures.
   >
   > Additional changes from the second draft:
   > - SPI functions aren't available during plperl.on_*_init execution.
   > - Added utility functions: quote_literal, quote_nullable, quote_ident,
   >    encode_bytea, decode_bytea, looks_like_number,
   >    encode_array_literal, encode_array_constructor.
   > - Enabled plperl to "use"/"require" safely by redirecting the require
   >    opcode to code that dies if module not already loaded.
   > - Corresponding changes to the documentation.
   >
   > Additional changes in this version:
   > - Added the missing ', arguments' to docs of spi_exec_prepared().
   > - Added Util.c to list of files for plperl make clean to delete.
   >
   > I'll add this to the commitfest.
   
   The rest of this all seems pretty nice, though I haven't read the patch yet.
   
   ...Robert
   
   

4. Re: Feature patch 1 for plperl [PATCH]

   Peter Eisentraut <peter_e@gmx.net> — 2010-01-09T21:13:58Z

   On fre, 2010-01-08 at 15:01 +0000, Tim Bunce wrote:
   > I didn't get any significant feedback from the earlier draft so here's
   > the finished 'feature patch 1' for plperl.
   
   I think it would help if you could split this up into about 6 to 10
   single-feature patches.
   
   
   

5. Re: Feature patch 1 for plperl [PATCH]

   Tim Bunce <tim.bunce@pobox.com> — 2010-01-09T21:40:22Z

   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.Bunce@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.
   
   

6. Re: Feature patch 1 for plperl [PATCH]

   Andrew Dunstan <andrew@dunslane.net> — 2010-01-10T00:14:12Z

   
   Peter Eisentraut wrote:
   > On fre, 2010-01-08 at 15:01 +0000, Tim Bunce wrote:
   >   
   >> I didn't get any significant feedback from the earlier draft so here's
   >> the finished 'feature patch 1' for plperl.
   >>     
   >
   > I think it would help if you could split this up into about 6 to 10
   > single-feature patches.
   >   
   
   I think that's a bit excessive. I'd suggest three patches:
   
       * the new utility functions (quote_literal, decode_bytea etc.) These
         should be fairly uncontroversial, but account for a large part of
         the patch volume.
       * the code relating to library load, interpreter initialization and
         termination
       * the remainder (function naming, better error checking, enabling
         use/require if a lib is already loaded, cleanup and optimization)
   
   We could ask Tim to break up the last, but they are all fairly small 
   items, and I at least wouldn't be bothered by having them combined. And 
   having to handle 6 to 10 patches all hitting the same body of code 
   doesn't sound terrible pleasant either.
   
   cheers
   
   andrew
   
   

7. Re: Feature patch 1 for plperl [PATCH]

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-10T06:27:20Z

   Andrew Dunstan <andrew@dunslane.net> writes:
   > Peter Eisentraut wrote:
   >> I think it would help if you could split this up into about 6 to 10
   >> single-feature patches.
   
   > ... having to handle 6 to 10 patches all hitting the same body of code 
   > doesn't sound terrible pleasant either.
   
   Indeed.  That sounds like rather a mess.
   
   			regards, tom lane
   
   

8. Re: Feature patch 1 for plperl [PATCH]

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-10T18:49:21Z

   Tim Bunce <Tim.Bunce@pobox.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.  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?
   
   I do not have a veto over stuff like this, but if I did, it would
   not go in.
   
   >> 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 ...
   
   			regards, tom lane
   
   

9. Re: Feature patch 1 for plperl [PATCH]

   Robert Haas <robertmhaas@gmail.com> — 2010-01-10T19:17:13Z

   On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
   > Tim Bunce <Tim.Bunce@pobox.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
   
   

10. Re: Feature patch 1 for plperl [PATCH]

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-10T19:49:20Z

    Robert Haas <robertmhaas@gmail.com> writes:
    > On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >> 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.
    
    shared_preload_libraries is under the sole control of the DBA, though.
    What distresses me about this is the exposure to ordinary users.
    In particular, that casual little "atexit" addition appears to mean
    that *unprivileged* users can break normal functioning of the database,
    eg by delaying or even preventing shutdown.  That's a security hole of
    the first magnitude.  Trying to execute SPI code there could make things
    even more fun.
    
    I also still don't care for the whole concept of on_init code from a
    theoretical standpoint: as I said before, 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.
    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.
    
    			regards, tom lane
    
    

11. Re: Feature patch 1 for plperl [PATCH]

    Andrew Dunstan <andrew@dunslane.net> — 2010-01-10T19:58:39Z

    
    Robert Haas wrote:
    > 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 don't know why you would do either of these things. I at least would 
    load one module which would in turn load others. So I'd expect to see 
    something like this:
    
        plperl.on_perl_init = 'use lib "/my/app"; use MyApp::Pg;'
    
    I think the suggestion that somehow people will want to put a huge list 
    of directives straight into postgresql.conf and that this is a reason 
    not to provide this facility is on the wrong track completely.
    
    > 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.
    
    cheers
    
    andrew
    
    

12. Re: Feature patch 1 for plperl [PATCH]

    Robert Haas <robertmhaas@gmail.com> — 2010-01-10T20:48:46Z

    On Sun, Jan 10, 2010 at 2:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Robert Haas <robertmhaas@gmail.com> writes:
    >> On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >>> 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.
    >
    > shared_preload_libraries is under the sole control of the DBA, though.
    > What distresses me about this is the exposure to ordinary users.
    > In particular, that casual little "atexit" addition appears to mean
    > that *unprivileged* users can break normal functioning of the database,
    > eg by delaying or even preventing shutdown.  That's a security hole of
    > the first magnitude.  Trying to execute SPI code there could make things
    > even more fun.
    
    That's really a separate issue from the on_perl_init stuff, but now
    that you mention it it does sound like a serious problem.  Preventing
    SPI from being executed there is probably feasible, but I don't like
    the idea that broken code would cause the database to fail to shut
    down, and that's probably not fixable (unless maybe we detach shared
    memory before executing this code, or something?).
    
    > I also still don't care for the whole concept of on_init code from a
    > theoretical standpoint: as I said before, 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.
    > 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.
    
    Hmm.  OK, I agree: that's a problem.
    
    ...Robert
    
    

13. Re: Feature patch 1 for plperl [PATCH]

    Andrew Dunstan <andrew@dunslane.net> — 2010-01-10T21:12:55Z

    
    Tom Lane wrote:
    > Robert Haas <robertmhaas@gmail.com> writes:
    >   
    >> On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >>     
    >>> 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.
    >>     
    >
    > shared_preload_libraries is under the sole control of the DBA, though.
    > What distresses me about this is the exposure to ordinary users.
    > In particular, that casual little "atexit" addition appears to mean
    > that *unprivileged* users can break normal functioning of the database,
    > eg by delaying or even preventing shutdown.  That's a security hole of
    > the first magnitude.  Trying to execute SPI code there could make things
    > even more fun.
    >   
    
    I suspect that could be inhibited at least.
    
    > I also still don't care for the whole concept of on_init code from a
    > theoretical standpoint: as I said before, 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.
    > 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.
    >
    > 			
    >   
    
    Well, I think the proposed plperl.on_perl_init setting could be 
    PGC_POSTMASTER, as I previously commented.
    
    The other settings are intended to be used on interpreter start, AIUI. 
    Maybe we need to explore the use cases more.
    
    If we made plperl.on_perl_init PGC_POSTMASTER and the other two 
    PGC_SUSET would that alloy your concerns?
    
    cheers
    
    andrew
    
    

14. Re: Feature patch 1 for plperl [PATCH]

    Robert Haas <robertmhaas@gmail.com> — 2010-01-10T21:35:46Z

    On Sun, Jan 10, 2010 at 2:58 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
    > I don't know why you would do either of these things. I at least would load
    > one module which would in turn load others. So I'd expect to see something
    > like this:
    >
    >   plperl.on_perl_init = 'use lib "/my/app"; use MyApp::Pg;'
    >
    > I think the suggestion that somehow people will want to put a huge list of
    > directives straight into postgresql.conf and that this is a reason not to
    > provide this facility is on the wrong track completely.
    
    Hmm.  I have to admit I didn't think about "use lib".  That does seem
    like a plausible thing to want to do.
    
    >> 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.
    
    ...Robert
    
    

15. Re: Feature patch 1 for plperl [PATCH]

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-10T22:12:16Z

    Andrew Dunstan <andrew@dunslane.net> writes:
    > Tom Lane wrote:
    >> 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.
    
    > If we made plperl.on_perl_init PGC_POSTMASTER and the other two 
    > PGC_SUSET would that alloy your concerns?
    
    No, they have to all be PGC_POSTMASTER to answer that concern.  Only
    breaking things for superusers isn't really that big an improvement
    over breaking them for everybody.
    
    			regards, tom lane
    
    

16. Re: Feature patch 1 for plperl [PATCH]

    Andrew Dunstan <andrew@dunslane.net> — 2010-01-10T23:18:23Z

    
    Tom Lane wrote:
    > Andrew Dunstan <andrew@dunslane.net> writes:
    >   
    >> Tom Lane wrote:
    >>     
    >>> 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.
    >>>       
    >
    >   
    >> If we made plperl.on_perl_init PGC_POSTMASTER and the other two 
    >> PGC_SUSET would that alloy your concerns?
    >>     
    >
    > No, they have to all be PGC_POSTMASTER to answer that concern.  Only
    > breaking things for superusers isn't really that big an improvement
    > over breaking them for everybody.
    >
    > 			
    >   
    
    Well, I don't know about Tim but I think I could live with that. And 
    when we get some actual experience with using them we'll have a better 
    handle on whether or not it gives us any pain.
    
    cheers
    
    andrew
    
    

17. Re: Feature patch 1 for plperl [PATCH]

    David E. Wheeler <david@kineticode.com> — 2010-01-10T23:44:31Z

    On Jan 10, 2010, at 11:17 AM, Robert Haas wrote:
    
    > 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;'
    
    Well, no, because sometimes I just want to load something and not have functions exported (into whatever namespaces ends up calling this). So I might have something like:
    
        plplerl.on_perl_init='use HTML::Entities ();'
    
    Other times I might want those functions exported.
    
    FWIW, Bricolage has a feature like this, and you can only put stuff on one line. It's been there since 2002 or so. No one has ever complained about it; I doubt anyone would complain about this, either.
    
    Best,
    
    David
    
    
    
    

18. Re: Feature patch 1 for plperl [PATCH]

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-11T00:05:47Z

    Andrew Dunstan <andrew@dunslane.net> writes:
    > Tom Lane wrote:
    >> No, they have to all be PGC_POSTMASTER to answer that concern.  Only
    >> breaking things for superusers isn't really that big an improvement
    >> over breaking them for everybody.
    
    > Well, I don't know about Tim but I think I could live with that. And 
    > when we get some actual experience with using them we'll have a better 
    > handle on whether or not it gives us any pain.
    
    Upon further review, PGC_POSTMASTER isn't going to work because of this
    in guc.c:
    
        /*
         * Only allow custom PGC_POSTMASTER variables to be created during shared
         * library preload; any later than that, we can't ensure that the value
         * doesn't change after startup.  This is a fatal elog if it happens; just
         * erroring out isn't safe because we don't know what the calling loadable
         * module might already have hooked into.
         */
        if (context == PGC_POSTMASTER &&
            !process_shared_preload_libraries_in_progress)
            elog(FATAL, "cannot create PGC_POSTMASTER variables after startup");
    
    We certainly don't want to make it such that plperl *has* to be
    preloaded, so PGC_POSTMASTER is out.  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.
    
    			regards, tom lane
    
    

19. Re: Feature patch 1 for plperl [PATCH]

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-11T00:26:01Z

    Tim Bunce <Tim.Bunce@pobox.com> writes:
    > 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.)
    
    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 state if an error
    occurs.
    
    Along the same line, there needs to be more effort put into the errors
    that can be thrown when one of these failures happen.  The current
    messages don't follow our style guidelines very well, and aren't exposed
    for translation.
    
    			regards, tom lane
    
    

20. Re: Feature patch 1 for plperl [PATCH]

    Robert Haas <robertmhaas@gmail.com> — 2010-01-13T00:06:59Z

    On Sun, Jan 10, 2010 at 4:35 PM, Robert Haas <robertmhaas@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 - 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.
    
    ...Robert
    
    

21. Re: Feature patch 1 for plperl [PATCH]

    Tim Bunce <tim.bunce@pobox.com> — 2010-01-13T11:30:13Z

    On Tue, Jan 12, 2010 at 07:06:59PM -0500, Robert Haas wrote:
    > On Sun, Jan 10, 2010 at 4:35 PM, Robert Haas <robertmhaas@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.