I'll look on it
From: Bruce Momjian <[EMAIL PROTECTED]>
To: Andrew Dunstan <[EMAIL PROTECTED]>
CC: Pavel Stehule <[EMAIL PROTECTED]>,
pgsql-patches@postgresql.org, [EMAIL PROTECTED]
Subject: Re: [PATCHES] better support of out parameters in plperl
Date: Thu, 8 Feb 2007 18:09:18 -0500 (EST)
This patch has been rejected based on comments just made by Andrew
Dunstan. If the author wants to revisit that, please reply and we can
discuss the issues.
---------------------------------------------------------------------------
Andrew Dunstan wrote:
>
>
> I wrote:
> > Pavel Stehule wrote:
> >> Hello,
> >>
> >> I send two small patches. First does conversion from perl to
> >> postgresql array in OUT parameters. Second patch allow hash form
> >> output from procedures with one OUT argument.
> >>
> >
> > I will try to review these in the next 2 weeks unless someone beats me
> > to it.
> >
> >
>
> I have reviewed this lightly, as committed by Bruce, and have some
> concerns. Unfortunately, the deathof my main workstation has cost me
> much of the time I intended to use for a more thorough review, so there
> may well be more issues than are outlined here.
>
> First, it is completely undocumented.
>
> Second, this comment is at best confusing:
>
> /* if value is ref on array do to pg string array conversion */
>
>
> Third, it appears to assume that we will have names for all OUT params.
But names are optional, as I understand it. Arguably, we should be treating
the returns positionally, and thus return an arrayref when there are OYT
params, not a hashref, and ignore the names - after all, all perl function
args are nameless, in fact, even if you use a naming convention to refer to
them.
>
> Fourth, I don't understand the change: "allow hash form output from
procedures with one OUT argument." That seems very non-orthogonal, and I
can't see any good reason for it.
>
> Lastly, if you look at the expected output as committed,it appears to
have been prepared without being actually examined, for example:
>
>
> CREATE OR REPLACE FUNCTION test05(OUT a varchar) AS $$
> return {a=>'ahoj'};
> $$ LANGUAGE plperl;
> SELECT '05' AS i,a FROM test05();
> i | a
> ----+-----------------
> 05 | HASH(0x8558f9c)
> (1 row)
>
>
> what???
>
> And now that I look I see every buildfarm box broken on PLCheck. That's
no surprise at all.
>
>
> The conversation regarding these features appears only to have started
on July 28th, which was probably much too late given some of the issues.
Unless we can solve these issues very fast I would be inclined to say this
should be tabled for 8.3. I think this is a fairly good illustration of the
danger of springing a feature, largely undiscussed, on the community just
about freeze time.
>
> cheers
>
> andrew
--
Bruce Momjian <[EMAIL PROTECTED]> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
_________________________________________________________________
Chcete sdilet sve obrazky a hudbu s prateli? http://messenger.msn.cz/
---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster