Re: better support of out parameters in plperl - Mailing list pgsql-patches

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
ahashref, and ignore the names - after all, all perl function args are nameless, in fact, even if you use a naming
conventionto refer to them. 
> >>>>
> >>>> Fourth, I don't understand the change: "allow hash form output from procedures with one OUT argument." That
seemsvery 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
lategiven some of the issues. Unless we can solve these issues very fast I would be inclined to say this should be
tabledfor 8.3. I think this is a fairly good illustration of the danger of springing a feature, largely undiscussed, on
thecommunity just about freeze time. 
> >>>>
> >>>> cheers
> >>>>
> >>>> andrew
> >>>>
> >>>>
> >>>
> >>>
> >
> >
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
>        subscribe-nomail command to majordomo@postgresql.org so that your
>        message can get through to the mailing list cleanly

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

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