PL/Perl list value return causes segfault

Michael Fuhr wrote:

In the latest HEAD, a PL/Perl function that returns a list value
instead of a reference causes a segmentation fault:

CREATE FUNCTION foo() RETURNS integer[] AS $$
return (1, 2, 3, 4);
$$ LANGUAGE plperl;

SELECT foo();
server closed the connection unexpectedly

Here's the stack trace:

#0 0xfed45bcc in plperl_call_handler (fcinfo=0xffbfe230) at plperl.c:1031
#1 0x0010e7d4 in ExecMakeFunctionResult (fcache=0x44af00, econtext=0x44ae58,
isNull=0x44b470 "\177~\177\177\177\177\177\177", isDone=0x44b4d8) at execQual.c:1031
#2 0x001122b0 in ExecProject (projInfo=0x44af00, isDone=0x44ae58) at execQual.c:3607

Patch below fixes the SEGV, and you will see instead:

andrew=# select foo();
ERROR: array value must start with "{" or dimension information

which might not immediately point you to the source of the error :-( ,
but is certainly better than a SEGV.

Note that all plperl functions are called in scalar context, and it is
always wrong to return a list (as opposed to a listref). In fact, the
value received might surprise you even if it worked (it would be the
value of the last member of the list).

cheers

andrew

Index: plperl.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/pl/plperl/plperl.c,v
retrieving revision 1.85
diff -c -r1.85 plperl.c
*** plperl.c    12 Jul 2005 01:16:21 -0000      1.85
--- plperl.c    12 Jul 2005 18:52:54 -0000
***************
*** 1021,1027 ****
          char *val;

!         if (prodesc->fn_retisarray && SvTYPE(SvRV(perlret)) == SVt_PVAV)
          {
              array_ret = plperl_convert_to_pg_array(perlret);
              SvREFCNT_dec(perlret);
--- 1021,1028 ----
          char *val;

! if (prodesc->fn_retisarray && SvROK(perlret) &&
! SvTYPE(SvRV(perlret)) == SVt_PVAV)
{
array_ret = plperl_convert_to_pg_array(perlret);
SvREFCNT_dec(perlret);

On Tue, Jul 12, 2005 at 02:59:37PM -0400, Andrew Dunstan wrote:

Note that all plperl functions are called in scalar context, and it is
always wrong to return a list (as opposed to a listref). In fact, the
value received might surprise you even if it worked (it would be the
value of the last member of the list).

Hmm, I don't know if it's feasible to do in Perl, but maybe check
whether the function wants to return something in list context and throw
an appropiate error message?

--
Alvaro Herrera (<alvherre[a]alvh.no-ip.org>)
"Nunca se desea ardientemente lo que solo se desea por raz�n" (F. Alexandre)

Alvaro Herrera wrote:

On Tue, Jul 12, 2005 at 02:59:37PM -0400, Andrew Dunstan wrote:

Note that all plperl functions are called in scalar context, and it is
always wrong to return a list (as opposed to a listref). In fact, the
value received might surprise you even if it worked (it would be the
value of the last member of the list).

Hmm, I don't know if it's feasible to do in Perl, but maybe check
whether the function wants to return something in list context and throw
an appropiate error message?

In perl, if there is any ambiguity it is the called function that is
responsible for checking, not the caller. See "perldoc -f wantarray".
PLPerl explicitly passed G_SCALAR as a flag on all calls to plperl
routines. So returning a list is a case of pilot error.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

Michael Fuhr wrote:

In the latest HEAD, a PL/Perl function that returns a list value
instead of a reference causes a segmentation fault:

Patch below fixes the SEGV, and you will see instead:

Applied, thanks.

regards, tom lane

On Tue, Jul 12, 2005 at 03:45:55PM -0400, Andrew Dunstan wrote:

Alvaro Herrera wrote:

On Tue, Jul 12, 2005 at 02:59:37PM -0400, Andrew Dunstan wrote:

Note that all plperl functions are called in scalar context, and
it is always wrong to return a list (as opposed to a listref). In
fact, the value received might surprise you even if it worked (it
would be the value of the last member of the list).

Hmm, I don't know if it's feasible to do in Perl, but maybe check
whether the function wants to return something in list context and
throw an appropiate error message?

In perl, if there is any ambiguity it is the called function that is
responsible for checking, not the caller. See "perldoc -f
wantarray". PLPerl explicitly passed G_SCALAR as a flag on all
calls to plperl routines. So returning a list is a case of pilot
error.

Is this a kind of pilot error that documents could help avert in some
useful way?

Cheers,
D
--
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778

Remember to vote!

David Fetter wrote:

On Tue, Jul 12, 2005 at 03:45:55PM -0400, Andrew Dunstan wrote:

In perl, if there is any ambiguity it is the called function that is
responsible for checking, not the caller. See "perldoc -f
wantarray". PLPerl explicitly passed G_SCALAR as a flag on all
calls to plperl routines. So returning a list is a case of pilot
error.

Is this a kind of pilot error that documents could help avert in some
useful way?

Sure. "A plperl function must always return a scalar value.More complex
structures (arrays, records, and sets) can be returned in the
appropriate context by returning a reference. A list should never be
returned." Salt to taste and insert where appropriate.

cheers

andrew

On Tue, Jul 12, 2005 at 02:59:37PM -0400, Andrew Dunstan wrote:

Note that all plperl functions are called in scalar context, and it is
always wrong to return a list (as opposed to a listref). In fact, the
value received might surprise you even if it worked (it would be the
value of the last member of the list).

Yeah, I knew that returning a list was contrary to what was expected, but
I wanted to see what would happen. I wasn't expecting a core dump :-(
Thanks for the patch.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

Would someone who knows perl update plperl.sgml and send me a patch?

Also, is this still true in 8.1:

In the current implementation, if you are fetching or returning
very large data sets, you should be aware that these will all go
into memory.

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

Andrew Dunstan wrote:

David Fetter wrote:

On Tue, Jul 12, 2005 at 03:45:55PM -0400, Andrew Dunstan wrote:

In perl, if there is any ambiguity it is the called function that is
responsible for checking, not the caller. See "perldoc -f
wantarray". PLPerl explicitly passed G_SCALAR as a flag on all
calls to plperl routines. So returning a list is a case of pilot
error.

Is this a kind of pilot error that documents could help avert in some
useful way?

Sure. "A plperl function must always return a scalar value.More complex
structures (arrays, records, and sets) can be returned in the
appropriate context by returning a reference. A list should never be
returned." Salt to taste and insert where appropriate.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Fri, Jul 29, 2005 at 11:24:37PM -0400, Bruce Momjian wrote:

Would someone who knows perl update plperl.sgml and send me a patch?

Also, is this still true in 8.1:

In the current implementation, if you are fetching or returning
very large data sets, you should be aware that these will all go
into memory.

That's no longer true. Please find enclosed a new patch :)

Cheers,
D
--
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778

Remember to vote!

David Fetter wrote:

*** 716,724 ****

<listitem>
<para>
!       In the current implementation, if you are fetching or returning
!       very large data sets, you should be aware that these will all go
!       into memory.
</para>
</listitem>
</itemizedlist>
--- 766,776 ----

<listitem>
<para>
! If you are fetching or returning very large data sets using
! <literal>spi_exec_query</literal>, you should be aware that
! these will all go into memory. You can avoid this by using
! <literal>spi_query</literal>/<literal>spi_fetchrow</literal> as
! illustrated earlier.
</para>
</listitem>
</itemizedlist>

You have rolled 2 problems into one - spi_query+spi_fetchrow does not
address the issue of returning large data sets.

Suggest instead:

<para>

If you are fetching very large data sets using
<literal>spi_exec_query</literal>, you should be aware that
these will all go into memory. You can avoid this by using
<literal>spi_query</literal> and <literal>spi_fetchrow</literal>
as illustrated earlier.
</para>
<para>
A similar problem occurs if a set-returning function passes
a large set of rows back to postgres via
<literal>return</literal>. You can avoid this
problem too by instead using <literal>return_next</literal> for
each row returned, as shown previously.
</para>

cheers

andrew

On Sat, Jul 30, 2005 at 09:47:58AM -0400, Andrew Dunstan wrote:

David Fetter wrote:

You have rolled 2 problems into one - spi_query+spi_fetchrow does not
address the issue of returning large data sets.

Suggest instead:

[suggestion]

Revised patch attached. Thanks for catching this :)

Cheers,
D
--
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778

Remember to vote!

Patch applied. Thanks.

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

David Fetter wrote:

On Sat, Jul 30, 2005 at 09:47:58AM -0400, Andrew Dunstan wrote:

David Fetter wrote:

You have rolled 2 problems into one - spi_query+spi_fetchrow does not
address the issue of returning large data sets.

Suggest instead:

[suggestion]

Revised patch attached. Thanks for catching this :)

Cheers,
D
--
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778

Remember to vote!

[ Attachment, skipping... ]

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

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073