Andrew Dunstan wrote:
> 
> Has it been resubmitted with issues attended to? If it has, I missed it. 
> If not, why should someone else waste time on something so broken that 
> it produced a stringified perl  hashref on output? It should never have 
> gone back in the queue IMNSHO.

Fine, removed.  I don't understand Perl well enough to judge that.

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


> 
> cheers
> 
> andrew
> 
> Bruce Momjian wrote:
> > 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
> >>>>     
> >>>>         
> >>>   
> >>>       
> >
> >   
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
>        subscribe-nomail command to [EMAIL PROTECTED] so that your
>        message can get through to the mailing list cleanly

-- 
  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 1: if posting/reading through Usenet, please send an appropriate
       subscribe-nomail command to [EMAIL PROTECTED] so that your
       message can get through to the mailing list cleanly

Reply via email to