return_next for plperl (was Re: call for help)
At 2005-05-21 20:18:50 +0530, ams@oryx.com wrote:
The second issue is where plperl returns a large result set.
I have attached the following seven patches to address this problem:
1. Trivial. Replaces some errant spaces with tabs.
2. Trivial. Fixes the spelling of Jan's name, and gets rid of many
inane, useless, annoying, and often misleading comments. Here's
a sample: "plperl_init_all() - Initialize all".
(I have tried to add some useful comments here and there, and will
continue to do so now and again.)
3. Trivial. Splits up some long lines.
4. Converts SRFs in PL/Perl to use a Tuplestore and SFRM_Materialize
to return the result set, based on the PL/PgSQL model.
There are two major consequences: result sets will spill to disk when
they can no longer fit in work_mem; and "select foo_srf()" no longer
works. (I didn't lose sleep over the latter, since that form is not
valid in PL/PgSQL, and it's not documented in PL/Perl.)
5. Trivial, but important. Fixes use of "undef" instead of undef. This
would cause empty functions to fail in bizarre ways. I suspect that
there's still another (old) bug here. I'll investigate further.
6. Moves the majority of (4) out into a new plperl_return_next()
function, to make it possible to expose the functionality to
Perl; cleans up some of the code besides.
7. Add an spi_return_next function for use in Perl code.
If you want to apply the patches and try them out, 8-composite.diff is
what you should use. (Note: my patches depend upon Andrew's use-strict
and %_SHARED patches being applied.)
Here's something to try:
create or replace function foo() returns setof record as $$
$i = 0;
for ("World", "PostgreSQL", "PL/Perl") {
spi_return_next({f1=>++$i, f2=>'Hello', f3=>$_});
}
return;
$$ language plperl;
select * from foo() as (f1 integer, f2 text, f3 text);
(Many thanks to Andrews Dunstan and Supernews for their help.)
Questions, comments, and suggestions welcome.
-- ams
Just Another Cut-and-Paste Hacker
Attachments:
1-tabs.difftext/plain; charset=us-asciiDownload+11-11
2-comments.difftext/plain; charset=us-asciiDownload+36-115
3-lines.difftext/plain; charset=us-asciiDownload+32-10
4-tuplestore.difftext/plain; charset=us-asciiDownload+90-147
5-undef.difftext/plain; charset=us-asciiDownload+1-1
6-retnext.difftext/plain; charset=us-asciiDownload+116-94
7-spiret.difftext/plain; charset=us-asciiDownload+11-4
8-composite.difftext/plain; charset=us-asciiDownload+240-283
Import Notes
Reply to msg id not found: 20050521144850.GA16465@penne.toroid.orgReference msg id not found: 428F4092.2050805@dunslane.netReference msg id not found: 20050521144850.GA16465@penne.toroid.org
On Sun, 2005-05-22 at 21:25 +0530, Abhijit Menon-Sen wrote:
I have attached the following seven patches to address this problem:
Does anyone with the skills to review this (i.e. someone other than me)
have any comments on this patch?
Otherwise I'll apply it in a day or two.
-Neil
Neil Conway said:
On Sun, 2005-05-22 at 21:25 +0530, Abhijit Menon-Sen wrote:
I have attached the following seven patches to address this problem:
Does anyone with the skills to review this (i.e. someone other than me)
have any comments on this patch?Otherwise I'll apply it in a day or two.
From recollection, my comments were these:
1. the exposed function should just be called return_next, not
spi_return_next2. either SPI.xs and friends should be renamed or all the non SPI code
should be moved into another .xs file.
3. regression sets need fixing, I think.
4. according to pg practice it should be done as context diffs, or perhaps
as one context diff ;-)
The patches appear to incorporate my 'use strict' enabling patch - that's a
good thing IMNSHO ;-) We can handle the custom var auth setting question
raised in my original submission of the feature later on.
cheers
andrew
cheers
andrew
Mega-patch version applied. Thanks.
---------------------------------------------------------------------------
Abhijit Menon-Sen wrote:
At 2005-05-21 20:18:50 +0530, ams@oryx.com wrote:
The second issue is where plperl returns a large result set.
I have attached the following seven patches to address this problem:
1. Trivial. Replaces some errant spaces with tabs.
2. Trivial. Fixes the spelling of Jan's name, and gets rid of many
inane, useless, annoying, and often misleading comments. Here's
a sample: "plperl_init_all() - Initialize all".(I have tried to add some useful comments here and there, and will
continue to do so now and again.)3. Trivial. Splits up some long lines.
4. Converts SRFs in PL/Perl to use a Tuplestore and SFRM_Materialize
to return the result set, based on the PL/PgSQL model.There are two major consequences: result sets will spill to disk when
they can no longer fit in work_mem; and "select foo_srf()" no longer
works. (I didn't lose sleep over the latter, since that form is not
valid in PL/PgSQL, and it's not documented in PL/Perl.)5. Trivial, but important. Fixes use of "undef" instead of undef. This
would cause empty functions to fail in bizarre ways. I suspect that
there's still another (old) bug here. I'll investigate further.6. Moves the majority of (4) out into a new plperl_return_next()
function, to make it possible to expose the functionality to
Perl; cleans up some of the code besides.7. Add an spi_return_next function for use in Perl code.
If you want to apply the patches and try them out, 8-composite.diff is
what you should use. (Note: my patches depend upon Andrew's use-strict
and %_SHARED patches being applied.)Here's something to try:
create or replace function foo() returns setof record as $$
$i = 0;
for ("World", "PostgreSQL", "PL/Perl") {
spi_return_next({f1=>++$i, f2=>'Hello', f3=>$_});
}
return;
$$ language plperl;
select * from foo() as (f1 integer, f2 text, f3 text);(Many thanks to Andrews Dunstan and Supernews for their help.)
Questions, comments, and suggestions welcome.
-- ams
Just Another Cut-and-Paste Hacker
[ Attachment, skipping... ]
[ Attachment, skipping... ]
[ Attachment, skipping... ]
[ Attachment, skipping... ]
[ Attachment, skipping... ]
[ Attachment, skipping... ]
[ Attachment, skipping... ]
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 9: 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 19073This has broken the regression tests for plperl - see for example
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=panda&dt=2005-06-04%2021:20:01
That's because, as Abhijit noted in point 4 below, select foo_srf() no
longer works.
At a minimum those calls need to be removed from the regression tests.
See also my later comments in response to Neil's request of a few days ago.
In particular, I would very much like the callback function renamed to a
simple "return_next".
cheers
andrew
Bruce Momjian said:
Show quoted text
Mega-patch version applied. Thanks.
---------------------------------------------------------------------------
Abhijit Menon-Sen wrote:
At 2005-05-21 20:18:50 +0530, ams@oryx.com wrote:
The second issue is where plperl returns a large result set.
I have attached the following seven patches to address this problem:
1. Trivial. Replaces some errant spaces with tabs.
2. Trivial. Fixes the spelling of Jan's name, and gets rid of many
inane, useless, annoying, and often misleading comments. Here's a
sample: "plperl_init_all() - Initialize all".(I have tried to add some useful comments here and there, and will
continue to do so now and again.)3. Trivial. Splits up some long lines.
4. Converts SRFs in PL/Perl to use a Tuplestore and SFRM_Materialize
to return the result set, based on the PL/PgSQL model.There are two major consequences: result sets will spill to disk
when they can no longer fit in work_mem; and "select foo_srf()" no
longer works. (I didn't lose sleep over the latter, since that form
is not valid in PL/PgSQL, and it's not documented in PL/Perl.)5. Trivial, but important. Fixes use of "undef" instead of undef. This
would cause empty functions to fail in bizarre ways. I suspect that
there's still another (old) bug here. I'll investigate further.6. Moves the majority of (4) out into a new plperl_return_next()
function, to make it possible to expose the functionality to
Perl; cleans up some of the code besides.7. Add an spi_return_next function for use in Perl code.
If you want to apply the patches and try them out, 8-composite.diff is
what you should use. (Note: my patches depend upon Andrew's use-strict
and %_SHARED patches being applied.)Here's something to try:
create or replace function foo() returns setof record as $$
$i = 0;
for ("World", "PostgreSQL", "PL/Perl") {
spi_return_next({f1=>++$i, f2=>'Hello', f3=>$_});
}
return;
$$ language plperl;
select * from foo() as (f1 integer, f2 text, f3 text);
Bruce Momjian said:
Andrew Dunstan wrote:
This has broken the regression tests for plperl - see for example
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=panda&dt=2005-06-04%2021:20:01>> That's because, as Abhijit noted in point 4 below, select foo_srf() no
longer works.
At a minimum those calls need to be removed from the regression tests.
See also my later comments in response to Neil's request of a few days
ago. In particular, I would very much like the callback function
renamed to a simple "return_next".OK, would you please submit a patch to fix it. Thanks.
I will unless someone beats me to it in the next 2 weeks - I am currently
travelling and don't really have development facilities available.
cheers
andrew
Import Notes
Reply to msg id not found: 200506042255.j54MtUi15377@candle.pha.pa.usReference msg id not found: 200506042255.j54MtUi15377@candle.pha.pa.us | Resolved by subject fallback
Andrew Dunstan wrote:
This has broken the regression tests for plperl - see for example
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=panda&dt=2005-06-04%2021:20:01
That's because, as Abhijit noted in point 4 below, select foo_srf() no
longer works.At a minimum those calls need to be removed from the regression tests.
See also my later comments in response to Neil's request of a few days ago.
In particular, I would very much like the callback function renamed to a
simple "return_next".
OK, would you please submit a patch to fix it. Thanks.
--
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 19073At 2005-06-04 17:27:10 -0500, andrew@dunslane.net wrote:
OK, would you please submit a patch to fix it. Thanks.
I will unless someone beats me to it in the next 2 weeks
Here's a patch to do the following:
1. Rename spi_return_next to return_next.
2. Add a new test for return_next.
3. Update the expected output.
4. Update the documentation.
-- ams
Attachments:
0text/plain; charset=us-asciiDownload+108-77
Patch applied. Thanks.
---------------------------------------------------------------------------
Abhijit Menon-Sen wrote:
At 2005-06-04 17:27:10 -0500, andrew@dunslane.net wrote:
OK, would you please submit a patch to fix it. Thanks.
I will unless someone beats me to it in the next 2 weeks
Here's a patch to do the following:
1. Rename spi_return_next to return_next.
2. Add a new test for return_next.
3. Update the expected output.
4. Update the documentation.-- ams
Content-Description: 1-rn-testdoc.diff
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
--
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