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
