Thread: PL/Perl: spi_prepare() and RETURNING

PL/Perl: spi_prepare() and RETURNING

From
David Fetter
Date:
Folks,

I've found a little lacuna in the RETURNING feature, that being in
PL/Perl's spi_prepare()/spi_execute_prepared() as illustrated in the
attached file on CVS TIP.

I think that fixing this is a matter of post-RETURNING-patch cleanup.
What else might not know about RETURNING that needs to?

Cheers,
D
-- 
David Fetter <david@fetter.org> http://fetter.org/
phone: +1 415 235 3778        AIM: dfetter666                             Skype: davidfetter

Remember to vote!


Re: PL/Perl: spi_prepare() and RETURNING

From
David Fetter
Date:
On Thu, Aug 24, 2006 at 11:31:02AM -0700, David Fetter wrote:
> Folks,
>
> I've found a little lacuna in the RETURNING feature, that being in
> PL/Perl's spi_prepare()/spi_execute_prepared() as illustrated in the
> attached file on CVS TIP.
>
> I think that fixing this is a matter of post-RETURNING-patch cleanup.
> What else might not know about RETURNING that needs to?
>
> Cheers,
> D


Oops.  This time, with the actual file attached :P

Cheers,
D
--
David Fetter <david@fetter.org> http://fetter.org/
phone: +1 415 235 3778        AIM: dfetter666
                              Skype: davidfetter

Remember to vote!

Attachment

Re: PL/Perl: spi_prepare() and RETURNING

From
Andrew Dunstan
Date:
David Fetter wrote:
> On Thu, Aug 24, 2006 at 11:31:02AM -0700, David Fetter wrote:
>   
>> Folks,
>>
>> I've found a little lacuna in the RETURNING feature, that being in
>> PL/Perl's spi_prepare()/spi_execute_prepared() as illustrated in the
>> attached file on CVS TIP.
>>
>> I think that fixing this is a matter of post-RETURNING-patch cleanup.
>> What else might not know about RETURNING that needs to?
>>
>> Cheers,
>> D
>>     
>
>
> Oops.  This time, with the actual file attached :P
>
>   
> my $foo_id = spi_exec_prepared(
>     $sth,
>     $_[0],
> )->{rows}->[0]->{foo_id};
>
>   

I am not sure that the 'rows' member is the right place for it, even if 
it were returned. For one thing, it should always be an arrayref, but 
overloading the first row like this is ugly.

Maybe 'returned_value' or some such would be better, e.g. $foo_id = 
$result->{returned_value}->{foo_id}

(We can't just stash it in the result hash with the used name in case 
somebody overrides the preexisiting members)

cheers

andrew



Re: PL/Perl: spi_prepare() and RETURNING

From
Tom Lane
Date:
David Fetter <david@fetter.org> writes:
>> I've found a little lacuna in the RETURNING feature, that being in
>> PL/Perl's spi_prepare()/spi_execute_prepared() as illustrated in the
>> attached file on CVS TIP.

It looks like plperl_spi_execute_fetch_result only expects to find
tuples returned if the status is SPI_OK_SELECT.  pltcl and plpython
have likely got similar issues.

This reminds me of a consideration I had been intending to bring up on
the mailing lists: what exactly do we want to do with the SPI API for
RETURNING queries?  The current behavior is that it still returns
SPI_OK_INSERT and so on, but also creates a SPI_tuptable.  Is this
what we want?  Perhaps we should invent additional return codes
SPI_OK_INSERT_RETURNING etc.

Another issue I noted in that same area is that spi.c does not set
SPI_processed for a utility statement, even if the utility statement
returns tuples.  Is this a bug, or should we leave it alone?
        regards, tom lane


Re: PL/Perl: spi_prepare() and RETURNING

From
Andrew Dunstan
Date:
Tom Lane wrote:
> Another issue I noted in that same area is that spi.c does not set
> SPI_processed for a utility statement, even if the utility statement
> returns tuples.  Is this a bug, or should we leave it alone?
>
>
>   

Bug or not, it's at least a limitation we could do without, I think.

cheers

andrew



Re: PL/Perl: spi_prepare() and RETURNING

From
"Jonah H. Harris"
Date:
On 8/24/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> This reminds me of a consideration I had been intending to bring up on
> the mailing lists: what exactly do we want to do with the SPI API for
> RETURNING queries?  The current behavior is that it still returns
> SPI_OK_INSERT and so on, but also creates a SPI_tuptable.  Is this
> what we want?  Perhaps we should invent additional return codes
> SPI_OK_INSERT_RETURNING etc.

I like adding RETURNING-specific return codes.

> Another issue I noted in that same area is that spi.c does not set
> SPI_processed for a utility statement, even if the utility statement
> returns tuples.  Is this a bug, or should we leave it alone?

I think it's a bug.

-- 
Jonah H. Harris, Software Architect | phone: 732.331.1300
EnterpriseDB Corporation            | fax: 732.331.1301
33 Wood Ave S, 2nd Floor            | jharris@enterprisedb.com
Iselin, New Jersey 08830            | http://www.enterprisedb.com/


Re: PL/Perl: spi_prepare() and RETURNING

From
Tom Lane
Date:
"Jonah H. Harris" <jonah.harris@gmail.com> writes:
> On 8/24/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> This reminds me of a consideration I had been intending to bring up on
>> the mailing lists: what exactly do we want to do with the SPI API for
>> RETURNING queries?

> I like adding RETURNING-specific return codes.

>> Another issue I noted in that same area is that spi.c does not set
>> SPI_processed for a utility statement, even if the utility statement
>> returns tuples.  Is this a bug, or should we leave it alone?

> I think it's a bug.

I've applied a patch along these lines.  David's plperl example now does
what (I think) he expected.  Does anyone want to extend the plperl,
pltcl, plpython regression tests to check behavior with INSERT RETURNING
etc?
        regards, tom lane


Re: PL/Perl: spi_prepare() and RETURNING

From
"Jonah H. Harris"
Date:
On 8/27/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I've applied a patch along these lines.  David's plperl example now does
> what (I think) he expected.

Kewl.  Thanks Tom.


-- 
Jonah H. Harris, Software Architect | phone: 732.331.1300
EnterpriseDB Corporation            | fax: 732.331.1301
33 Wood Ave S, 2nd Floor            | jharris@enterprisedb.com
Iselin, New Jersey 08830            | http://www.enterprisedb.com/