Would someone review this. It is in the patches_hold queue:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
---------------------------------------------------------------------------
Andrew Dunstan wrote:
>
> I think it has to wait to 8.3. It's a complete mess that was submitted
> unheralded at the last moment. Pavel needs to get into the habit of
> submitting ideas first, not just patches. And there must be proper
> documentation and working regression tests.
>
> cheers
>
> andrew
>
> Bruce Momjian wrote:
> > Uh, were are we in fixing/reviewing this?
> >
> > ---------------------------------------------------------------------------
> >
> > 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 6: explain analyze is your friend
