[HACKERS] user-based query white list

[Tim Burgan]

This old thread on "user-based query white list" is now nearly 10 years old!
http://grokbase.com/t/postgresql/pgsql-hackers/08c6zh42fa/user-based-query-white-list

Since then, is it now possible to configure a user to only be able to
execute a limited white-listing of queries? Is this something that could
now be implemented through extensions?

Re: [HACKERS] tsearch_extras extension

[Tim Abbott]

Just following up here since I haven't gotten a reply -- I'd love to work
with someone from the Postgres community on a plan to make the
tsearch_extras functionality available as part of mainline postgres.


-Tim Abbott

On Wed, Feb 3, 2016 at 9:41 PM, Tim Abbott <tabb...@mit.edu> wrote:

> Hello,
>
> I'm a maintainer of the Zulip open source group chat application.  Zulip
> depends on a small (~200 lines) postgres extension called tsearch_extras (
> https://github.com/zbenjamin/tsearch_extras) that returns the actual
> (offset, length) pairs of all the matches for a postgres full text search
> query.  This extension allows Zulip to do its own highlighting of the full
> text search matches, using a more complicated method than what Postgres
> supports natively.
>
> I think tsearch_extras is probably of value to others using postgres
> full-text search (and I'd certainly love to get out of the business of
> maintaining an out-of-tree postgres extension), so I'm wondering if this
> feature (or a variant of it) would be of interest to upstream?
>
> Thanks!
>
> -Tim Abbott
>
> (See
> http://www.postgresql.org/message-id/flat/52c7186d.8010...@strangersgate.com#52c7186d.8010...@strangersgate.com
> for the discussion on postgres mailing lists that caused us to write this
> module in the first place.)
>
>

Re: [HACKERS] CREATE INDEX CONCURRENTLY?

[Tim Kane]

This just hit us today... Admittedly on an old cluster still running 9.2,
though I can't see any mention of it being addressed since.

Any chance of getting this on to to-do list?
On Sat, 1 Nov 2014 at 07:45, Simon Riggs  wrote:

> On 31 October 2014 17:46, Michael Banck  wrote:
>
> > I wonder whether that is pilot error (fair enough), or whether something
> > could be done about this?
>
> When originally written the constraints were tighter, but have since
> been relaxed.
>
> Even so a CIC waits until all snapshots that can see it have gone. So
> what you observe is correct and known.
>
>
> Can it be changed? Maybe.
>
> CREATE INDEX gets around the wait by using indcheckxmin to see whether
> the row is usable. So the command completes, even if the index is not
> usable by all current sessions.
>
> We perform the wait in a completely different way for CIC, for this
> reason (in comments)
>
>   We also need not set indcheckxmin during a concurrent index build,
>   because we won't set indisvalid true until all transactions that care
>   about the broken HOT chains are gone.
>
> Reading that again, I can't see why we do it that way. If CREATE INDEX
> can exit once the index is built, so could CONCURRENTLY.
>
> ISTM that we could indcheckxmin into an Xid, not a boolean
>For CREATE INDEX, set the indcheckxmin = xid of creating transaction
>For CREATE INDEX CONCURRENTLY set the indcheckxmin = xid of the
> completing transaction
>
> The apparent reason it does this is that the Xmin value used currently
> is the Xmin of the index row. The index row is inserted prior to the
> index being valid so that technique cannot work. So I am suggesting
> for CIC that we use the xid of the transaction that completes the
> index, not the xid that originally created the index row. Plus handle
> the difference between valid and not.
>
> --
>  Simon Riggs   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>
>
> --
> 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] postgres_fdw foreign keys with default sequence

[Tim Kane]

Slight typo on my local host example there.  s/clone/local/
More like the below:


CREATE FOREIGN TABLE IF NOT EXISTS live.devices (
 device_id  bigint NOT NULL
 );

CREATE MATERIALISED VIEW local.devices;

CREATE test_table (device_id bigint FOREIGN KEY (device_id) REFERENCES
*local*.devices (device_id) );

ERROR:  referenced relation devices is not a table

On Tue, Feb 17, 2015 at 1:08 PM, Tim Kane tim.k...@gmail.com wrote:

 Hi all,

 Not sure if this has been reported already, it seems to be a variation on
 this thread:

 http://www.postgresql.org/message-id/20130515151059.go4...@tamriel.snowman.net


 One minor difference is, in my scenario - my source table field is defined
 as BIGINT (not serial) - though it does have a default nextval on a
 sequence, so ultimately - the same dependence.

 The primary difference (IMHO), is that I am actually foreign keying on a
 local materialised view of the fdw'ed foreign table.



 On the foreign host:
   Table live.devices
Column   |  Type  | Modifiers

 ++---
  device_id  | bigint | not null default
 nextval('devices_id_sequence'::regclass)


 On the local host:


 CREATE FOREIGN TABLE IF NOT EXISTS live.devices (
  device_id  bigint NOT NULL
  );

 CREATE MATERIALISED VIEW local.devices;

 CREATE test_table (device_id bigint FOREIGN KEY (device_id) REFERENCES
 clone.devices (device_id) );

 ERROR:  referenced relation devices is not a table



 Though this is a similar scenario to the previous thread, I would have
 expected foreign keying from a materialised view to behave independently of
 the FDW, as if from a regular local table.

 FYI, I'm running postgresql 9.3.4

 Cheers,

 Tim

[HACKERS] postgres_fdw foreign keys with default sequence

[Tim Kane]

Hi all,

Not sure if this has been reported already, it seems to be a variation on
this thread:

http://www.postgresql.org/message-id/20130515151059.go4...@tamriel.snowman.net


One minor difference is, in my scenario - my source table field is defined
as BIGINT (not serial) - though it does have a default nextval on a
sequence, so ultimately - the same dependence.

The primary difference (IMHO), is that I am actually foreign keying on a
local materialised view of the fdw'ed foreign table.



On the foreign host:
  Table live.devices
   Column   |  Type  | Modifiers
++---
 device_id  | bigint | not null default
nextval('devices_id_sequence'::regclass)


On the local host:


CREATE FOREIGN TABLE IF NOT EXISTS live.devices (
 device_id  bigint NOT NULL
 );

CREATE MATERIALISED VIEW local.devices;

CREATE test_table (device_id bigint FOREIGN KEY (device_id) REFERENCES
clone.devices (device_id) );

ERROR:  referenced relation devices is not a table



Though this is a similar scenario to the previous thread, I would have
expected foreign keying from a materialised view to behave independently of
the FDW, as if from a regular local table.

FYI, I'm running postgresql 9.3.4

Cheers,

Tim

Re: [HACKERS] [GENERAL] Migrating from 9.2.4 to 9.3.0 with XML DOCTYPE

[Tim Kane]

 
 
 From:  Tom Lane t...@sss.pgh.pa.us
 
 Hm, can you restore it into 9.2 either?
 
 AFAICS, pg_dump has absolutely no idea that it should be worried about the
 value of xmloption, despite the fact that that setting affects what is
 considered valid XML data.  What's worse, even if it were attempting to do
 something about xmloption, I don't see how it could deal with a case like
 this where you have different values inside the same database that require
 two different settings in order to parse.
 
 This isn't a 9.3.x bug, it's an aboriginal misdesign of the XML datatype.
 Not sure what we can do about it at this point.  Perhaps we could invent
 a document_or_content setting that would tell xml_in to accept either
 case?  And then have pg_dump force that setting to be used during restore?


This sounds reasonable. My use case is purely as a document store, with the
ability to perform xml parse functions against it – as such, I’m not
concerned wether it’s a document or content – hence why we have both types
recorded against that field.

For the minute, I’m getting around the restore problem by mangling the dump
such that the table is created using the text type rather than xml.  This at
least gets the data onto a 9.3 cluster, even if it’s cosmetically
represented as text instead of xml.  I can worry about the document vs
content problem at a later stage.

 
 PS: BTW, I agree with the advice expressed by David J: under no
 circumstances put any data you care about on 9.3.0.  That release
 was rather a disaster from a quality-control standpoint :-(
 But that's unrelated to your XML issue.


Ack. Thanks for the info. I’ll push the upgrade-path agenda a little harder.

Re: [HACKERS] removing old ports and architectures

[Tim Kane]

Just to be pedantic, commit message shows
support for Tru64 ended in 201.

I think you mean 2012.





On 18/10/2013 13:41, Robert Haas robertmh...@gmail.com wrote:

On Thu, Oct 17, 2013 at 5:41 PM, Peter Eisentraut pete...@gmx.net wrote:
 On 10/17/13 12:45 PM, Robert Haas wrote:
 The attached patch, which I propose to apply relatively soon if nobody
 objects, removes the IRIX port.

 +1

Done.  And here's a patch for removing the alpha architecture and
Tru64 UNIX (aka OSF/1) which runs on that architecture, per discussion
upthread.  Barring objections, I'll apply this next week.

On a related note, I think we should update the paragaraph in
installation.sgml that begins In general, PostgreSQL can be expected
to work on these CPU architectures.  Any architecture that doesn't
have a buildfarm animal should be relegated to the second sentence,
which reads Code support exists for ... but these architectures are
not known to have been tested recently.  Similarly, I think the
following paragraph should be revised so that only operating systems
for which we have current buildfarm support are considered fully
supported.  Others should be relegated to a sentence later in the
paragraph that says something like code support exists but not tested
recently or expected to work but not tested regularly.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] Suggestion for concurrent index creation using a single full scan operation

[Tim Kane]

Wow.. thanks guys, really appreciate the detailed analysis.

Tim


On Wed, Jul 24, 2013 at 4:08 AM, Noah Misch n...@leadboat.com wrote:

 On Tue, Jul 23, 2013 at 01:06:26PM +0100, Tim Kane wrote:
  I haven't given this a lot of thought, but it struck me that when
  rebuilding tables (be it for a restore process, or some other operational
  activity) - there is more often than not a need to build an index or two,
  sometimes many indexes, against the same relation.
 
  It strikes me that in order to build just one index, we probably need to
  perform a full table scan (in a lot of cases).   If we are building
  multiple indexes sequentially against that same table, then we're
 probably
  performing multiple sequential scans in succession, once for each index.

 Check.

  Could we architect a mechanism that allowed multiple index creation
  statements to execute concurrently, with all of their inputs fed directly
  from a single sequential scan against the full relation?
 
  From a language construct point of view, this may not be trivial to
  implement for raw/interactive SQL - but possibly this is a candidate for
  the custom format restore?

 As Greg Stark mentioned, pg_restore can already issue index build commands
 in
 parallel.  Where applicable, that's probably superior to having one backend
 build multiple indexes during a single heap scan.  Index builds are
 CPU-intensive, and the pg_restore approach takes advantage of additional
 CPU
 cores in addition to possibly saving I/O.

 However, the pg_restore method is not applicable if you want CREATE INDEX
 CONCURRENTLY, and it's not applicable for implicit index building such as
 happens for ALTER TABLE rewrites and for VACUUM FULL.  Backend-managed
 concurrent index builds could shine there.

  I presume this would substantially increase the memory overhead required
 to
  build those indexes, though the performance gains may be advantageous.

 The multi-index-build should respect maintenance_work_mem overall.
  Avoiding
 cases where that makes concurrent builds slower than sequential builds is a
 key challenge for such a project:

 - If the index builds each fit in maintenance_work_mem when run
 sequentially
   and some spill to disk when run concurrently, expect concurrency to lose.
 - If the heap is small enough to stay in cache from one index build to the
   next, performing the builds concurrently is probably a wash or a loss.
 - Concurrency should help when a wide-row table large enough to exhaust OS
   cache has narrow indexes that all fit in maintenance_work_mem.  I don't
 know
   whether concurrency would help for a huge-table scenario where the
 indexes
   do overspill maintenance_work_mem.  You would have N indexes worth of
   external merge files competing for disk bandwidth; that could cancel out
   heap I/O savings.

 Overall, it's easy to end up with a loss.  We could punt by having an
 index_build_concurrency GUC, much like pg_restore relies on the user to
 discover a good -j value.  But if finding cases where concurrency helps
 is
 too hard, leaving the GUC at one would become the standard advice.

  Apologies in advance if this is not the correct forum for suggestions..

 It's the right forum.

 Thanks,
 nm

 --
 Noah Misch
 EnterpriseDB http://www.enterprisedb.com

[HACKERS] Suggestion for concurrent index creation using a single full scan operation

[Tim Kane]

Hi all,

I haven't given this a lot of thought, but it struck me that when
rebuilding tables (be it for a restore process, or some other operational
activity) - there is more often than not a need to build an index or two,
sometimes many indexes, against the same relation.

It strikes me that in order to build just one index, we probably need to
perform a full table scan (in a lot of cases).   If we are building
multiple indexes sequentially against that same table, then we're probably
performing multiple sequential scans in succession, once for each index.

Could we architect a mechanism that allowed multiple index creation
statements to execute concurrently, with all of their inputs fed directly
from a single sequential scan against the full relation?

From a language construct point of view, this may not be trivial to
implement for raw/interactive SQL - but possibly this is a candidate for
the custom format restore?

I presume this would substantially increase the memory overhead required to
build those indexes, though the performance gains may be advantageous.

Feel free to shoot holes through this :)

Apologies in advance if this is not the correct forum for suggestions..

Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

[Tim Bunce]

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] vacuumlo patch

[Tim]

Excerpts from Josh's message On Sun, Aug 7, 2011 at 2:36 AM:

 could we figure out what that limit should be based on
 max_locks_per_transaction?


It would be nice to implement via -l max instead of making users do it
manually or something like this -l $(grep max_locks_per_transaction.*=
postgresql.conf | perl -p -e 's/#.*//g;s/^.*?([0-9]+).*?$/$1/g' | grep .
|head -1 ).
For this patch I just want to enable the limit functionality, leaving higher
level options like max to the user for now.


 handle deleting all the ophan LOs in several transactions for the user
 automatically?


I addressed this option before and basically said it is an undesirable
alternative because It is a less flexible option that is easily implemented
in a shell script.
Again it would be a welcome extra option but it can be left to the user for
now.

Re: [HACKERS] vacuumlo patch

[Tim]

Excerpts from Peter's message On Sun, Aug 7, 2011 at 3:49 AM:

 Please put the new option 'l' in some sensible order in the code and the
 help output (normally alphabetical).  Also, there should probably be
 some update to the documentation.


I have alphabetized the help output, and one piece of code.
I'm hesitate to alphabetize some portions of the code because they are
grouped by type and not already alphabetized.
If I left something un-alphabetized please be explicit about about what
should be changed and why.
Documentation is now also in the patch.
Thanks for the tips.


vacuumlo.patch
Description: Binary data

-- 
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

[Tim Bunce]

:
 
 Let's slow down a bit. Nobody that we know of has encountered the
 problem Tom's referring to, over all the years plperlu has been
 available. The changes you're proposing have the potential to
 downgrade the usefulness of plperlu considerably without fixing
 anything that's known to be an actual problem. Instead of fixing a
 problem caused by using LWP you could well make LWP totally unusable
 from plperlu.
 
 And it still won't do a thing about signal handlers installed by C code.
 
 And plperlu would be the tip of the iceberg. What about all the
 other PLs, not to mention non-PL loadable modules?
 
 But we *can* fix the original problem reported, namely failure to
 restore signal handlers on function exit, with very little downside
 (assuming it's shown to be fairly cheap).

On Thu, Aug 04, 2011 at 09:23:49PM -0600, Alex Hunsaker wrote:
 
  And plperlu would be the tip of the iceberg. What about all the other PLs,
  not to mention non-PL loadable modules?
 
 Maybe the answer is to re-issue the appropriate pqsignals() instead of
 doing the perl variant?
 
 For PL/Perl(u) we could still disallow any signals the postmaster
 uses, from my quick look that would be: HUP, INT, TERM, QUIT, ALRM,
 PIPE, USR1, USR2, FPE. All we would need to do is restore ALRM.
 
 Or am I too paranoid about someone shooting themselves in the foot via
 USR1? (BTW you can set signals in plperl, but you can't call alarm()
 or kill())

On Fri, Aug 05, 2011 at 10:53:21AM -0400, Andrew Dunstan wrote:
 
 This whole thing is a massive over-reaction to a problem we almost
 certainly know how to fix fairly simply and relatively painlessly,
 and attempts (unsuccessfully, at least insofar as comprehensiveness
 is concerned) to fix a problem nobody's actually reported having
 AFAIK.

For plperl, as Alex noted above, kill() and alarm() can't be used but
%SIG can be altered. Locally making %SIG readonly for plperl subs
(after __DIE__ and __WARN__ are added) seems cheap and effective.

For plperlu, clearly $SIG{ALRM} is useful. Enforcing localization, thus
fixing the immediate problem, and documenting that it won't work
reliably with statement_timeout, seems like a reasonable approach.

plperlu is already a potential footgun in countless ways. Documenting
that other signal handlers, like USR1, shouldn't be used ought to be enough.

On Sat, Aug 06, 2011 at 12:37:28PM -0600, Alex Hunsaker wrote:
 
 *shrug* OK.
 
 Find attached a version that does the equivalent of local %SIG for
 each pl/perl(u) call.

 + gv = gv_fetchpv(SIG, 0, SVt_PVHV);
 + save_hash(gv);  /* local %SIG */

After a little digging and some discussion on the #p5p channel [thanks
to ilmari++ leont++ and sorear++ for their help] it seems that local(%SIG)
doesn't do what you might expect. The %SIG does become empty but the OS
level handlers, even those installed by perl, *aren't changed*:

$ perl -wE '$SIG{INT} = sub { say Foo}; { local %SIG; kill INT, $$; };'
Foo

And, even worse, they're not reset at scope exit:

$ perl -wE '$SIG{INT} = sub { say Foo}; { local %SIG; $SIG{INT} = sub {say 
Bar }} kill INT, $$;'
Bar

That sure seems like a bug (I'll check with the perl5-porters list).

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);
}

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] vacuumlo patch

[Tim]

Thanks Josh,
I like the ability to bail out on PQTRANS_INERROR, and I think it's a small
enough fix to be appropriate to include in this patch.
I did consider it before but did not implement it because I am still new to
pgsql-hackers and did not know how off-the-cuff.
So thanks for the big improvement.

Re: [HACKERS] vacuumlo patch

[Tim]

Hi Josh,

Thanks for help. Attached is a patch including changes suggested in your
comments.

Excerpts from Josh's message On Sat, Aug 6, 2011 at 9:57 PM:


  1. It wasn't clear to me whether you're OK with Aron's suggested
 tweak, please include it in your patch if so.


I decided to and included Aron's tweak.
I'm not sure if the PostgreSQL project prefers simplified code over faster
run time (if only under rare conditions).
Who knows maybe the compiler will re-optimize it anyway.

  2. I think it might be better to use INT_MAX instead of hardcoding
 2147483647.


Fixed, though I'm not sure if I put the include in the preferred order.


   3. ... should have a space before the first '(' and around the '!=' and
 '='


Fixed.


   4. The rest of the usage strings spell out 'large object(s)' instead
 of abbreviating 'LO'...


Fixed, I omitted the brackets around the s to conform with the other usage
strings.


   5. transaction_limit is an int, yet you're using strtol() which
 returns long. Maybe you want to use atoi() or make transaction_limit a
 long?


The other INT in the code is using strtol() so I also used strtol instead of
changing more code.
I'm not sure if there are any advantages or disadvantages.
maybe it's easier to prevent/detect/report overflow wraparound.

I can't think of a good reason anyone would want to limit transaction to
something more than INT_MAX.
Implementing that would best be done in separate and large patch as
PQntuples returns an int and is used on 271 lines across PostgreSQL.


Thanks again for the help.
2147483647


vacuumlo.patch
Description: Binary data

-- 
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] vacuumlo patch

[Tim Lewis]

Hi Aron,

Thanks for the input. The small change you suggest would change the
behavior of the patch which I would prefer not to do as I have reasons for
the previous behavior.
Because you gave no reasons and stop after removing LIMIT LOs was not
changed to stop after attempting to remove LIMIT LOs I suspect you were
just trying to keep the code clean.

The difference:
In your version of the patch vacuumlo will stop after N lo_unlink(OID)
attempts.
The previous behavior of the patch is that vacuumlo will stop after N
successful lo_unlink(OID)s.

If you have good reason for your behavior please add another flag so that it
is optional.
There should be a clear distinction between counting vs not, and aborting
vs continuing when a lo_unlink(OID) is unsuccessful.


On Tue, Jul 26, 2011 at 4:18 AM, Aron m...@eunice.de wrote:

 Here's another small change to the patch, it works fine for me and it quite
 saved my day.

 I try to submit the patch by email.


 diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
 index f6e2a28..1f88d72 100644
 --- a/contrib/vacuumlo/vacuumlo.c
 +++ b/contrib/vacuumlo/vacuumlo.c
 @@ -48,6 +48,7 @@ struct _param
char   *pg_host;
int verbose;
int dry_run;
 +   int transaction_limit;
  };

  intvacuumlo(char *, struct _param *);
 @@ -286,6 +287,8 @@ vacuumlo(char *database, struct _param * param)
 }
else
deleted++;
 +   if(param-transaction_limit!=0 
 deleted=param-transaction_limit)
 +   break;
}
PQclear(res);

 @@ -313,6 +316,7 @@ usage(const char *progname)
 printf(  -h HOSTNAME  database server host or socket
 directory\n);
printf(  -n   don't remove large objects, just show what
 would be
 done\n);
printf(  -p PORT  database server port\n);
 +   printf(  -l LIMIT stop after removing LIMIT LOs\n);
printf(  -U USERNAME  user name to connect as\n);
printf(  -w   never prompt for password\n);
printf(  -W   force password prompt\n);
 @@ -342,6 +346,7 @@ main(int argc, char **argv)
 param.pg_port = NULL;
param.verbose = 0;
param.dry_run = 0;
 +   param.transaction_limit = 0;

if (argc  1)
{
 @@ -359,7 +364,7 @@ main(int argc, char **argv)

while (1)
{
 -   c = getopt(argc, argv, h:U:p:vnwW);
 +   c = getopt(argc, argv, h:U:p:l:vnwW);
if (c == -1)
break;

 @@ -395,6 +400,14 @@ main(int argc, char **argv)
 }
param.pg_port = strdup(optarg);
break;
 +   case 'l':
 +   param.transaction_limit = strtol(optarg,
 NULL, 10);
 +   if ((param.transaction_limit  0) ||
 (param.transaction_limit 
 2147483647))
 +   {
 +   fprintf(stderr, %s: invalid
 transaction limit number: %s, valid
 range is form 0(disabled) to 2147483647.\n, progname, optarg);
 +   exit(1);
 +   }
 +   break;
case 'h':
param.pg_host = strdup(optarg);
break;


 --
 View this message in context:
 http://postgresql.1045698.n5.nabble.com/vacuumlo-patch-tp4628522p4634026.html
 Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

 --
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers




-- 

Noodle
Connecting People, Content  Capabilities within the Enterprise


Toll Free: 866-258-6951 x 701
tim.le...@vialect.com
http://www.vialect.com

Noodle is a product of Vialect Inc

Follow Noodle on Twitter
http://www.twitter.com/noodle_news

Re: [HACKERS] vacuumlo patch

[Tim]

Updated the patch to also apply when the no-action flag is enabled.

git diff HEAD -- contrib/vacuumlo/vacuumlo.c
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index f6e2a28..8e9c342 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -48,6 +48,7 @@ struct _param
 char   *pg_host;
 intverbose;
 intdry_run;
+inttransaction_limit;
 };

 intvacuumlo(char *, struct _param *);
@@ -282,10 +283,18 @@ vacuumlo(char *database, struct _param * param)
 fprintf(stderr, %s, PQerrorMessage(conn));
 }
 else
+{
 deleted++;
+if(param-transaction_limit!=0 
deleted=param-transaction_limit)
+break;
+}
 }
 else
+{
 deleted++;
+if(param-transaction_limit!=0 
deleted=param-transaction_limit)
+break;
+}
 }
 PQclear(res);

@@ -313,6 +322,7 @@ usage(const char *progname)
 printf(  -h HOSTNAME  database server host or socket directory\n);
 printf(  -n   don't remove large objects, just show what would
be done\n);
 printf(  -p PORT  database server port\n);
+printf(  -l LIMIT stop after removing LIMIT LOs\n);
 printf(  -U USERNAME  user name to connect as\n);
 printf(  -w   never prompt for password\n);
 printf(  -W   force password prompt\n);
@@ -342,6 +352,7 @@ main(int argc, char **argv)
 param.pg_port = NULL;
 param.verbose = 0;
 param.dry_run = 0;
+param.transaction_limit = 0;

 if (argc  1)
 {
@@ -359,7 +370,7 @@ main(int argc, char **argv)

 while (1)
 {
-c = getopt(argc, argv, h:U:p:vnwW);
+c = getopt(argc, argv, h:U:p:l:vnwW);
 if (c == -1)
 break;

@@ -395,6 +406,14 @@ main(int argc, char **argv)
 }
 param.pg_port = strdup(optarg);
 break;
+case 'l':
+param.transaction_limit = strtol(optarg, NULL, 10);
+if ((param.transaction_limit  0) ||
(param.transaction_limit  2147483647))
+{
+fprintf(stderr, %s: invalid transaction limit number:
%s, valid range is form 0(disabled) to 2147483647.\n, progname, optarg);
+exit(1);
+}
+break;
 case 'h':
 param.pg_host = strdup(optarg);
 break;

[HACKERS] vacuumlo patch

[Tim]

Please consider adding this minor patch, or offering improvements.

*Problem*: vacuumlo required PostgreSQL to use more locks per transaction
than possible resulting in an error and a log file full of ignored until
end of transaction warnings.
(max_locks_per_transaction is limited by shmmax which is limited by RAM)

*Solution*: ask vacuumlo to complete after N lo_unlink(OID)s.
*
Alternate Solution*: ask vacuumlo to start a new transaction after N
lo_unlink(OID)s.
(more complex and can be implemented with the other solution in the shell)

*Design Decisions*: Add a flag so the new behavior is optional and it
defaults to the old behavior.

*Implementation*: Followed code formatting style.
It's likely pointless to check if an int is  2147483647 but maybe it
clarifies the code.
Added a friendly error message and the a line in the help.

git diff -U0 HEAD -- contrib/vacuumlo/vacuumlo.c
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index f6e2a28..b7c7d64 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -50,0 +51 @@ struct _param
+inttransaction_limit;
@@ -284,0 +286 @@ vacuumlo(char *database, struct _param * param)
+{
@@ -285,0 +288,5 @@ vacuumlo(char *database, struct _param * param)
+if(param-transaction_limit!=0 
deleted=param-transaction_limit)
+{
+break;
+}
+}
@@ -315,0 +323 @@ usage(const char *progname)
+printf(  -l LIMIT stop after removing LIMIT LOs\n);
@@ -344,0 +353 @@ main(int argc, char **argv)
+param.transaction_limit = 0;
@@ -362 +371 @@ main(int argc, char **argv)
-c = getopt(argc, argv, h:U:p:vnwW);
+c = getopt(argc, argv, h:U:p:l:vnwW);
@@ -397,0 +407,8 @@ main(int argc, char **argv)
+case 'l':
+param.transaction_limit = strtol(optarg, NULL, 10);
+if ((param.transaction_limit  0) ||
(param.transaction_limit  2147483647))
+{
+fprintf(stderr, %s: invalid transaction limit number:
%s, valid range is form 0(disabled) to 2147483647.\n, progname, optarg);
+exit(1);
+}
+break;

Re: [HACKERS] vacuumlo patch

[Tim]

Hi Álvaro, thanks for the response.

Here is the requested diff with 3 lines of context:

git diff HEAD -- contrib/vacuumlo/vacuumlo.c
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index f6e2a28..b7c7d64 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -48,6 +48,7 @@ struct _param
 char   *pg_host;
 intverbose;
 intdry_run;
+inttransaction_limit;
 };

 intvacuumlo(char *, struct _param *);
@@ -282,7 +283,13 @@ vacuumlo(char *database, struct _param * param)
 fprintf(stderr, %s, PQerrorMessage(conn));
 }
 else
+{
 deleted++;
+if(param-transaction_limit!=0 
deleted=param-transaction_limit)
+{
+break;
+}
+}
 }
 else
 deleted++;
@@ -313,6 +320,7 @@ usage(const char *progname)
 printf(  -h HOSTNAME  database server host or socket directory\n);
 printf(  -n   don't remove large objects, just show what would
be done\n);
 printf(  -p PORT  database server port\n);
+printf(  -l LIMIT stop after removing LIMIT LOs\n);
 printf(  -U USERNAME  user name to connect as\n);
 printf(  -w   never prompt for password\n);
 printf(  -W   force password prompt\n);
@@ -342,6 +350,7 @@ main(int argc, char **argv)
 param.pg_port = NULL;
 param.verbose = 0;
 param.dry_run = 0;
+param.transaction_limit = 0;

 if (argc  1)
 {
@@ -359,7 +368,7 @@ main(int argc, char **argv)

 while (1)
 {
-c = getopt(argc, argv, h:U:p:vnwW);
+c = getopt(argc, argv, h:U:p:l:vnwW);
 if (c == -1)
 break;

@@ -395,6 +404,14 @@ main(int argc, char **argv)
 }
 param.pg_port = strdup(optarg);
 break;
+case 'l':
+param.transaction_limit = strtol(optarg, NULL, 10);
+if ((param.transaction_limit  0) ||
(param.transaction_limit  2147483647))
+{
+fprintf(stderr, %s: invalid transaction limit number:
%s, valid range is form 0(disabled) to 2147483647.\n, progname, optarg);
+exit(1);
+}
+break;
 case 'h':
 param.pg_host = strdup(optarg);
 break;

Re: [HACKERS] [BUGS] BUG #6034: pg_upgrade fails when it should not.

[Tim Uckun]

 I thought the problem was that they upgraded the OS and now the encoding
 names changed, though they behaved the same.  Is that now what is
 happening?  Can they supply the values with different cases?



In my case I never touched the locale. It was set by the OS. I presume
this is true for most people.

-- 
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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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

Re: [HACKERS] SHOW TABLES

[Tim Landscheidt]

Simon Riggs si...@2ndquadrant.com wrote:

 [...]
 Light switches are usually at shoulder height next to a door. Our light
 switches are 2 metres up, on the far side of the room. People are sick
 of banging their knees on furniture while trying to grope for the light.
 The light switch isn't so much hard to use, its just in the wrong place.
 We must envisage what it is to be a person that doesn't know where the
 switch is, or have forgotten. We don't need a programmable light switch
 API, or a multi-function light remote control. Just a switch by all of
 the doors.

 (Oh, they're probably not called lights outside UK; room lamps maybe?)

Wow, the British must have shrunk a lot since my last vis-
it - here light switches are mounted not more than 105 cm
from the floor :-) (barrier-free not more than 85 cm).

  I guess the problem shown by others in this thread is that
there doesn't seem to be a usually with regard to \d
equivalents either.

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] SHOW TABLES

[Tim Landscheidt]

Kevin Grittner kevin.gritt...@wicourts.gov wrote:

  postgres=# SHOW ME THE MONEY;
 WARNING: THE MONEY is deprecated in this version of Postgres and
 may be discarded in a future version
 HINT: Use SHOW ME THE NUMERIC with the desired precision instead.

 Funny, but no longer true:

 http://www.postgresql.org/docs/8.4/static/datatype-money.html

 (although I wish we would get rid of the type)

 I hadn't been aware it was ever deprecated.  It has the advantage
 over numeric of using straight integer arithmetic for addition and
 subtraction, which are by far the most common operations on money,
 while allowing a decimal fraction without rounding problems.  I'd
 been thinking about migrating our money columns to it (subject to
 some benchmarking first, to see how much it actually helped).  It
 would seem odd for a database to tout its ability to deal with such
 data types as geometric shapes and global positioning, etc., which
 then didn't have such a common type as money.  In my experience,
 many business applications deal with money.

One major flaw I see is that the fractional precision is
fixed. Not only petrol stations split cents.

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] cvs to git migration - keywords

[Tim Landscheidt]

Tom Lane t...@sss.pgh.pa.us wrote:

 1) We can migrate the repository with the keywords, and then make one big
 commit just after (or before, that doesn't make a difference) removing
 them. In this case, backbranches and tags look exactly like they do
 now, but it also means if you do git diff between old versions, the
 keywords will show up there.

 +1 for #1. Changing history and the resulting possibility of becoming
 one's own grandfather always makes me nervous.

 Yeah.  One concrete problem with removing the $PostgreSQL$ lines is it
 will affect line numbering everywhere.  Yeah, it's only off-by-one, but
 there could still be confusion.
 [...]

If not the whole line was removed, but only the
$PostgreSQL$ part, the numbering should stay the same. I
guess it would otherwise be challenging to automatically not
only delete the $PostgreSQL$ line, but also leading and/or
trailing empty (comment) lines, and not mess up.

Tim


-- 
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)

[Tim Bunce]

[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

[Tim Bunce]

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]

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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]

[Tim Bunce]

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)

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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

[Tim Bunce]

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 = call

[HACKERS] Re: PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

[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

[Tim Bunce]

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]

[Tim Bunce]

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

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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 a/src/pl/plperl

Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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??

[Tim Bunce]

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

[Tim Bunce]

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]

[Tim Bunce]

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]

[Tim Bunce]

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