Re: [PATCHES] WIP 2 interpreters for plperl

I have made some progress with what I think is needed to have two
interpreters for plperl. This is a lot harder than the pltcl case for
two reasons: 1. there are no restrictions on having 2 tcl interpreters,
and 2. tcl does not need to save and restore context as we have to do
with perl. I think I have a conceptual siolution to these two problems,
but what I have is currently segfaulting somewhat myteriously. Tracing a
dynamically loaded library in a postgres backend with a debugger is less
than fun, too. I am attaching what I currently have, liberally sprinkled
with elog(NOTICE) calls as trace writes.

I need to get some other work done today too, so I will return to this
later if I can. In the meanwhile, if anybody cares to cast a fresh set
of eyeballs over this, please do.

cheers

andrew

I wrote:
>
> I have made some progress with what I think is needed to have two
> interpreters for plperl. This is a lot harder than the pltcl case for
> two reasons: 1. there are no restrictions on having 2 tcl
> interpreters, and 2. tcl does not need to save and restore context as
> we have to do with perl. I think I have a conceptual siolution to
> these two problems, but what I have is currently segfaulting somewhat
> myteriously. Tracing a dynamically loaded library in a postgres
> backend with a debugger is less than fun, too. I am attaching what I
> currently have, liberally sprinkled with elog(NOTICE) calls as trace
> writes.
>
>

With a little more perseverance I found the problem. The attached patch
passes regression. But it now needs plenty of eyeballs and testing.

cheers

andrew

[moving to -hackers]

I wrote:
>
>>
>> I have made some progress with what I think is needed to have two
>> interpreters for plperl. This is a lot harder than the pltcl case for
>> two reasons: 1. there are no restrictions on having 2 tcl
>> interpreters, and 2. tcl does not need to save and restore context as
>> we have to do with perl. I think I have a conceptual siolution to
>> these two problems, but what I have is currently segfaulting somewhat
>> myteriously. Tracing a dynamically loaded library in a postgres
>> backend with a debugger is less than fun, too. I am attaching what I
>> currently have, liberally sprinkled with elog(NOTICE) calls as trace
>> writes.
>>
>>
>
> With a little more perseverance I found the problem. The attached
> patch passes regression. But it now needs plenty of eyeballs and testing.
>
>

Well, if anyone cast eyeballs over it they kept it secret from me :-(

However, I have now tested the patch with the little script shown below
and it seems to do the Right Thing (tm) in switching context and
restoring it. So I think it can be applied to HEAD, along with an
addition to the docs and a release note.

Since this is a behaviour modification, do we want to apply it to the
back branches? Doing so would certainly be possible, although it would
be non-trivial.

cheers

andrew

------------

drop function if exists f1(int);
drop function if exists f2(int);

create function f1(int) returns void language plperl as
$$

my $arg = shift;
elog NOTICE,"in plperl func f1($arg)";
return if ($arg > 5);
$arg++;
spi_exec_query("select f2($arg)");

$$;

create function f2(int) returns void language plperlu as
$$

my $arg = shift;
elog NOTICE,"in plperlu func f2($arg)";
return if ($arg > 5);
$arg++;
spi_exec_query("select f1($arg)");

$$;

select f1(0);
select f2(0);

I wrote:
>
> [moving to -hackers]
>
> I wrote:
>>
>>>
>>> I have made some progress with what I think is needed to have two
>>> interpreters for plperl. This is a lot harder than the pltcl case
>>> for two reasons: 1. there are no restrictions on having 2 tcl
>>> interpreters, and 2. tcl does not need to save and restore context
>>> as we have to do with perl. I think I have a conceptual siolution to
>>> these two problems, but what I have is currently segfaulting
>>> somewhat myteriously. Tracing a dynamically loaded library in a
>>> postgres backend with a debugger is less than fun, too. I am
>>> attaching what I currently have, liberally sprinkled with
>>> elog(NOTICE) calls as trace writes.
>>>
>>>
>>
>> With a little more perseverance I found the problem. The attached
>> patch passes regression. But it now needs plenty of eyeballs and
>> testing.
>>
>>
>
> Well, if anyone cast eyeballs over it they kept it secret from me :-(
>
>
> However, I have now tested the patch with the little script shown
> below and it seems to do the Right Thing (tm) in switching context and
> restoring it. So I think it can be applied to HEAD, along with an
> addition to the docs and a release note.
>
> Since this is a behaviour modification, do we want to apply it to the
> back branches? Doing so would certainly be possible, although it would
> be non-trivial.
>

I have committed this to HEAD at any rate, so that we can get some
buildfarm testing going.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> Since this is a behaviour modification, do we want to apply it to the
>> back branches? Doing so would certainly be possible, although it would
>> be non-trivial.

> I have committed this to HEAD at any rate, so that we can get some
> buildfarm testing going.

My vote is to leave it just in HEAD; there may be someone out there
depending on plperl and plperlu being in the same interpreter, and
breaking their code in a minor release doesn't seem very friendly.

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>>> Since this is a behaviour modification, do we want to apply it to the
>>> back branches? Doing so would certainly be possible, although it would
>>> be non-trivial.
>>>
>
>
>> I have committed this to HEAD at any rate, so that we can get some
>> buildfarm testing going.
>>
>
> My vote is to leave it just in HEAD; there may be someone out there
> depending on plperl and plperlu being in the same interpreter, and
> breaking their code in a minor release doesn't seem very friendly.
>
>
>

Fine by me.

cheers

andrew

I might be one of the ones who depends on the same interpreter. In your new
scheme, the _SHARED hash will only be shared between like interpreters,
correct? This is going to force me to switch all of my perl code to use the
plperlu interpreter :(

-----Original Message-----
From: pgsql-hackers-owner(at)postgresql(dot)org
[mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Tom Lane
Sent: Monday, November 20, 2006 2:10 PM
To: Andrew Dunstan
Cc: PostgreSQL-development
Subject: Re: [HACKERS] [PATCHES] WIP 2 interpreters for plperl

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> Since this is a behaviour modification, do we want to apply it to the
>> back branches? Doing so would certainly be possible, although it would
>> be non-trivial.

> I have committed this to HEAD at any rate, so that we can get some
> buildfarm testing going.

My vote is to leave it just in HEAD; there may be someone out there
depending on plperl and plperlu being in the same interpreter, and
breaking their code in a minor release doesn't seem very friendly.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
message can get through to the mailing list cleanly

Jim Buttafuoco wrote:
> I might be one of the ones who depends on the same interpreter. In your new
> scheme, the _SHARED hash will only be shared between like interpreters,
> correct? This is going to force me to switch all of my perl code to use the
> plperlu interpreter :(
>
>

Yes. Sorry, but I can't see any way around it. If anyone can suggest one
then speak up loudly ASAP.

cheers

andrew

On Mon, Nov 20, 2006 at 04:14:34PM -0500, Andrew Dunstan wrote:
> Jim Buttafuoco wrote:
> >I might be one of the ones who depends on the same interpreter. In your
> >new
> >scheme, the _SHARED hash will only be shared between like interpreters,
> >correct? This is going to force me to switch all of my perl code to use
> >the
> >plperlu interpreter :(
>
> Yes. Sorry, but I can't see any way around it. If anyone can suggest one
> then speak up loudly ASAP.

Since the stuff plperlu should be small and self contained, you just
need to set it up so all the data needed by the plperlu function is
passed as a parameter. I suppose we'd need to look at the use case to
see if this is a real obsticle.

I suppose you're not permitted to call other perl functions directly
with \%_SHARED as a parameter, right?

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Martijn van Oosterhout wrote:
> On Mon, Nov 20, 2006 at 04:14:34PM -0500, Andrew Dunstan wrote:
>
>> Jim Buttafuoco wrote:
>>
>>> I might be one of the ones who depends on the same interpreter. In your
>>> new
>>> scheme, the _SHARED hash will only be shared between like interpreters,
>>> correct? This is going to force me to switch all of my perl code to use
>>> the
>>> plperlu interpreter :(
>>>
>> Yes. Sorry, but I can't see any way around it. If anyone can suggest one
>> then speak up loudly ASAP.
>>
>
> Since the stuff plperlu should be small and self contained, you just
> need to set it up so all the data needed by the plperlu function is
> passed as a parameter. I suppose we'd need to look at the use case to
> see if this is a real obsticle.
>
> I suppose you're not permitted to call other perl functions directly
> with \%_SHARED as a parameter, right?
>
>
>

\%_SHARED only has meaning in the context of a given perl interpreter.
If we use it in another interpreter it will point to the middle of
nowhere. It's the equivalent of one program passing a pointer to another
program. I thought of playing clever games with a tied interface
(perldoc perltie for more info), but then we'd still have troubles with
things like:

my $xxx=2;
$_SHARED{foo} = { bar => [1,2,3], baz=> sub { return ++$xxx; } };

The only thing I have seen that looked remotely promising is the
non-standard Safe::World module, which if it lives up to its promise
might allow us to go back to using a single interpreter. But I have not
had time to investigate further, and I don't thing we can rely on a
module almost no standard installation will have, unless we want to ship
it ourselves. It doesn't seem to have been worked on since 2004. It is
certainly too late to think of anything like that for 8.2, I think - it
would need significant analysis and testing which I do not currently
have time for, and release is just around the corner, we fervently hope.

cheers

andrew