Thread: PL/Perl list value return causes segfault
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 -- Michael Fuhr http://www.fuhr.org/~mfuhr/
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
thesewill 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,
Pennsylvania19073
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!
Attachment
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!
Attachment
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