Like I said, at the last moment.


Bruce Momjian wrote:
Oh, let me add that this was first discussed on July 28:

        http://archives.postgresql.org/pgsql-hackers/2006-07/msg01421.php

and a patch posted on July 30:

        http://archives.postgresql.org/pgsql-hackers/2006-07/msg01559.php

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

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 9: In versions below 8.0, the planner will ignore your desire to
      choose an index scan if your joining column's datatypes do not
      match

Reply via email to