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

Reply via email to