Re: Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]

On Sat, Jan 30, 2010 at 08:49, Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> wrote:
> 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

Im not a fan of the names (I think everyone gets trusted vs untrusted
confused). May I humbly suggest:
plperl.on_init
plperlu.on_init
plperl.both_on_init <- this one is the one that throws the scheme off :(

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

> - select_perl_context() state management improved
>    An error during interpreter initialization will leave
>    the state (interp_state etc) unchanged.

This looked good.

> - 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?
Looks good to me otherwise.

The tests dont seem to pass :( this is from a make installcheck-world
test plperl_shared ... FAILED
...
test plperl_init ... FAILED

with:
SET plperl.on_trusted_init = '$_SHARED{on_init} = 42';
+ ERROR: unrecognized configuration parameter "plperl.on_trusted_init"
-- test the shared hash

If I throw a LOAD 'plperl'; at the top of those sql files it works...

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 <literal>plperl</> 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" ?