Re: plperl db access documentation enhancement

Hello,

We were asked by Enova Financial to improve the documentation of PL/Perl database access functions.
Alvaro and me worked on that and we produced the patch that is attached. It splits initial block of functions
into the groups with the description directly following each of the group, corrects couple of mistakes and
adds an example.

One of the existing mistakes was confusion in definitions of spi_exec_prepared and spi_query_prepared.
Another one is usage of INTEGER type to return the result of spi_prepare in the example for prepared queries.
When trying to execute that function I've got the following error:

postgres=# CREATE OR REPLACE FUNCTION init() RETURNS INTEGER AS $#
$_SHARED{my_plan} = spi_prepare( 'SELECT (now() + $1)::date AS now', 'INTERVAL');
$$ LANGUAGE plperl;

CREATE FUNCTION

postgres=# select init();
ERROR: invalid input syntax for integer: "0x1007d6f40"
CONTEXT: PL/Perl function "init"

Since the return value is not used anyway, I've changed the return type of the function declaration in the example to VOID.

I think this is a good reason to suggest backpatching these changes down to 8.2.

Patch applied for 9.0. We don't normally backpatch such documentation
improvements unless we receive multiple reports of confusion.

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

Alexey Klyukin wrote:
> Hello,
>
> We were asked by Enova Financial to improve the documentation of PL/Perl database access functions.
> Alvaro and me worked on that and we produced the patch that is attached. It splits initial block of functions
> into the groups with the description directly following each of the group, corrects couple of mistakes and
> adds an example.
>
> One of the existing mistakes was confusion in definitions of spi_exec_prepared and spi_query_prepared.
> Another one is usage of INTEGER type to return the result of spi_prepare in the example for prepared queries.
> When trying to execute that function I've got the following error:
>
> postgres=# CREATE OR REPLACE FUNCTION init() RETURNS INTEGER AS $#
> $_SHARED{my_plan} = spi_prepare( 'SELECT (now() + $1)::date AS now', 'INTERVAL');
> $$ LANGUAGE plperl;
>
> CREATE FUNCTION
>
> postgres=# select init();
> ERROR: invalid input syntax for integer: "0x1007d6f40"
> CONTEXT: PL/Perl function "init"
>
> Since the return value is not used anyway, I've changed the return type of the function declaration in the example to VOID.
>
> I think this is a good reason to suggest backpatching these changes down to 8.2.
>
>

[ Attachment, skipping... ]

>
>
> --
> Alexey Klyukin http://www.CommandPrompt.com/
> The PostgreSQL Company - Command Prompt, Inc
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
>
> Patch applied for 9.0. We don't normally backpatch such documentation
> improvements unless we receive multiple reports of confusion.

I think that's a mistake in this case. The documentation wasn't
confusing -- it was bogus. (Actually, the bug fixing is a smaller
change than the whole of this patch, so we could provide the smaller
patch if desired to apply to 8.4. To be honest I think it is better to
just apply the larger patch verbatim.)

I could do the backpatch if you want. I just wanted some more peer
review on the changes.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:
> Bruce Momjian wrote:
> >
> > Patch applied for 9.0. We don't normally backpatch such documentation
> > improvements unless we receive multiple reports of confusion.
>
> I think that's a mistake in this case. The documentation wasn't
> confusing -- it was bogus. (Actually, the bug fixing is a smaller
> change than the whole of this patch, so we could provide the smaller
> patch if desired to apply to 8.4. To be honest I think it is better to
> just apply the larger patch verbatim.)
>
> I could do the backpatch if you want. I just wanted some more peer
> review on the changes.

Sure, go ahead if you are sure. I wasn't clear enough to risk it, and
documentation churn in back branches has its own downsides, which is why
I avoid it, especially for larger patches.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Alvaro Herrera wrote:
> > Bruce Momjian wrote:
> > >
> > > Patch applied for 9.0. We don't normally backpatch such documentation
> > > improvements unless we receive multiple reports of confusion.
> >
> > I think that's a mistake in this case. The documentation wasn't
> > confusing -- it was bogus. (Actually, the bug fixing is a smaller
> > change than the whole of this patch, so we could provide the smaller
> > patch if desired to apply to 8.4. To be honest I think it is better to
> > just apply the larger patch verbatim.)
> >
> > I could do the backpatch if you want. I just wanted some more peer
> > review on the changes.
>
> Sure, go ahead if you are sure. I wasn't clear enough to risk it, and
> documentation churn in back branches has its own downsides, which is why
> I avoid it, especially for larger patches.

Just applied it.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support