Unix Technical Forum

SEO

vBulletin Search Engine Optimization


Go Back   Unix Technical Forum > Database Server Software > PostgreSQL > Pgsql Patches

Register FAQ Members List Calendar Search Today's Posts Mark Forums Read
  #1 (permalink)  
Old 04-18-2008, 08:56 AM
Bruce Momjian
 
Posts: n/a
Default Re: better support of out parameters in plperl


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 bruce@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 majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #2 (permalink)  
Old 04-18-2008, 08:57 AM
Andrew Dunstan
 
Posts: n/a
Default Re: better support of out parameters in plperl


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 4: Have you searched our list archives?

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #3 (permalink)  
Old 04-18-2008, 08:57 AM
Bruce Momjian
 
Posts: n/a
Default Re: better support of out parameters in plperl


OK.

This has been saved for the 8.3 release:

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 bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

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

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #4 (permalink)  
Old 04-18-2008, 08:57 AM
Bruce Momjian
 
Posts: n/a
Default Re: better support of out parameters in plperl


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

http://archives.postgresql.org/pgsql...7/msg01421.php

and a patch posted on July 30:

http://archives.postgresql.org/pgsql...7/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
> >>

> >
> >


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

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

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #5 (permalink)  
Old 04-18-2008, 08:57 AM
Andrew Dunstan
 
Posts: n/a
Default Re: better support of out parameters in plperl


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...7/msg01421.php
>
> and a patch posted on July 30:
>
> http://archives.postgresql.org/pgsql...7/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

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #6 (permalink)  
Old 04-18-2008, 08:57 AM
Tom Lane
 
Posts: n/a
Default Re: better support of out parameters in plperl

Bruce Momjian <bruce@momjian.us> writes:
> Uh, were are we in fixing/reviewing this?


It's dead for 8.2 --- Andrew's complaints are pretty serious at both
the conceptual and implementation levels, and there's been no sign of
discussion about how to fix them.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #7 (permalink)  
Old 04-18-2008, 09:29 AM
Bruce Momjian
 
Posts: n/a
Default Re: better support of out parameters in plperl


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 <bruce@momjian.us> 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

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #8 (permalink)  
Old 04-18-2008, 09:29 AM
Andrew Dunstan
 
Posts: n/a
Default Re: better support of out parameters in plperl


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.

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 majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #9 (permalink)  
Old 04-18-2008, 09:29 AM
Bruce Momjian
 
Posts: n/a
Default Re: better support of out parameters in plperl

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 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. +

---------------------------(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

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
Reply


Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On
Forum Jump


All times are GMT. The time now is 04:32 AM.


Powered by vBulletin® Version 3.6.5
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
SEO by vBSEO 3.2.0
UnixAdminTalk.com

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265