Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-14 Thread Heikki Linnakangas
On 14.12.2011 12:31, Yeb Havinga wrote: On 2011-12-13 18:34, Tom Lane wrote: Heikki Linnakangas writes: Attached is a patch with those changes. I also I removed a few of the syntax error regression tests, that seemed excessive, plus some general naming and comment fiddling. I'll apply this tomo

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-14 Thread Yeb Havinga
On 2011-12-13 18:34, Tom Lane wrote: Heikki Linnakangas writes: Attached is a patch with those changes. I also I removed a few of the syntax error regression tests, that seemed excessive, plus some general naming and comment fiddling. I'll apply this tomorrow, if it still looks good to me after

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-13 Thread Tom Lane
Heikki Linnakangas writes: > Attached is a patch with those changes. I also I removed a few of the > syntax error regression tests, that seemed excessive, plus some general > naming and comment fiddling. I'll apply this tomorrow, if it still looks > good to me after sleeping on it. The code lo

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-13 Thread Heikki Linnakangas
On 12.12.2011 21:55, Kevin Grittner wrote: Yeb Havinga wrote: Forgot to copy regression output to expected - attached v7 fixes that. This version addresses all of my concerns. It applies cleanly and compiles without warning against current HEAD and performs as advertised. I'm marking it Re

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-12 Thread Kevin Grittner
Yeb Havinga wrote: > Forgot to copy regression output to expected - attached v7 fixes > that. This version addresses all of my concerns. It applies cleanly and compiles without warning against current HEAD and performs as advertised. I'm marking it Ready for Committer. -Kevin -- Sent via

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-12 Thread Yeb Havinga
On 2011-12-11 16:26, Yeb Havinga wrote: On 2011-12-06 17:58, Kevin Grittner wrote: Kevin Grittner wrote: Yeb Havinga wrote: I personally tend to believe it doesn't even need to be an error. There is no technical reason not to allow it. All the user needs to do is make sure that the combina

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-11 Thread Yeb Havinga
On 2011-12-06 17:58, Kevin Grittner wrote: Kevin Grittner wrote: Yeb Havinga wrote: I personally tend to believe it doesn't even need to be an error. There is no technical reason not to allow it. All the user needs to do is make sure that the combination of named parameters and the position

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-07 Thread Yeb Havinga
On 2011-12-06 17:58, Kevin Grittner wrote: Kevin Grittner wrote: If there are no objections, I suggest that Yeb implement the mixed notation for cursor parameters. Hearing no objections -- Yeb, are you OK with doing this, and do you feel this is doable for this CF? It is not a large change,

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-06 Thread Kevin Grittner
Kevin Grittner wrote: > Yeb Havinga wrote: >> I personally tend to believe it doesn't even need to be an error. >> There is no technical reason not to allow it. All the user needs >> to do is make sure that the combination of named parameters and >> the positional ones together are complete and

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-05 Thread Kevin Grittner
"Kevin Grittner" wrote: > Yeb Havinga wrote: >> I personally tend to believe it doesn't even need to be an error. >> There is no technical reason not to allow it. All the user needs >> to do is make sure that the combination of named parameters and >> the positional ones together are complete a

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-05 Thread Kevin Grittner
Yeb Havinga wrote: > On 2011-12-03 21:53, Kevin Grittner wrote: >> (1) Doc changes: >> >>(a) These contain some unnecessary whitespace changes which >>make it harder to see exactly what changed. > > This is probably caused because I used emacs's fill-paragraph > (alt+q) command, a

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-05 Thread Yeb Havinga
Kevin, Thank you for your review! On 2011-12-03 21:53, Kevin Grittner wrote: (1) Doc changes: (a) These contain some unnecessary whitespace changes which make it harder to see exactly what changed. This is probably caused because I used emacs's fill-paragraph (alt+q) command, af

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-03 Thread Kevin Grittner
The patch is in context format, includes docs and additional regression tests, applies cleanly, passes "world" regression tests. The parser changes don't cause any "backing up". Discussion on the list seems to have reached a consensus that this patch implements a useful feature. A previous versi

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-11-15 Thread Yeb Havinga
On 2011-11-14 15:45, Yeb Havinga wrote: On 2011-10-15 07:41, Tom Lane wrote: Yeb Havinga writes: Hello Royce, Thanks again for testing. I looked this patch over but concluded that it's not ready to apply, mainly because there are too many weird behaviors around error reporting. Thanks again

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-11-14 Thread Yeb Havinga
On 2011-10-15 07:41, Tom Lane wrote: Yeb Havinga writes: Hello Royce, Thanks again for testing. I looked this patch over but concluded that it's not ready to apply, mainly because there are too many weird behaviors around error reporting. Thanks again for the review and comments. Attached is

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-17 Thread Yeb Havinga
On 2011-10-15 07:41, Tom Lane wrote: Yeb Havinga writes: Hello Royce, Thanks again for testing. I looked this patch over but concluded that it's not ready to apply, mainly because there are too many weird behaviors around error reporting. Tom, thanks for reviewing - getting the syntax errors

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-14 Thread Tom Lane
Yeb Havinga writes: > Hello Royce, > Thanks again for testing. I looked this patch over but concluded that it's not ready to apply, mainly because there are too many weird behaviors around error reporting. The biggest problem is that the patch cuts up and reassembles the source text and then han

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-11 Thread Royce Ausburn
On 11/10/2011, at 11:38 PM, Yeb Havinga wrote: > Declaration of cursors with named parameters is already part of PostgreSQL > (so it is possible to use the parameter names in the cursor query instead of > $1, $2, etc.) and it also already documented with an example, just a few > lines above th

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-11 Thread Yeb Havinga
Hello Royce, Thanks again for testing. On 2011-10-11 13:55, Royce Ausburn wrote: Just one small thing: it'd be nice to have an example for cursor declaration with named parameters. Your patch adds one for opening a cursor, but not for declaring one. Declaration of cursors with named param

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-11 Thread Royce Ausburn
On 08/10/2011, at 1:56 AM, Yeb Havinga wrote: > Attach is v2 of the patch. > > Mixed notation now raises an error. > > In contrast with what I said above, named parameter related errors are thrown > before any syntax errors. I tested with raising syntax errors first but the > resulting code w

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-07 Thread Yeb Havinga
On 2011-10-07 12:21, Yeb Havinga wrote: On 2011-10-06 16:04, Royce Ausburn wrote: Initial Review for patch: http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php Again, thank you very much for your thorough review. I'll update the patch so mixing positional and named parameters

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-07 Thread Yeb Havinga
On 2011-10-06 16:04, Royce Ausburn wrote: Initial Review for patch: http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php Hello Royce, Thank you for your review. I don't think so. The new feature accepts opening a cursor with some parameter names not specified: open c

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Royce Ausburn
Forgive my ignorance -- do I need to be doing anything else now seeing as I started the review? On 07/10/2011, at 7:15 AM, Pavel Stehule wrote: > 2011/10/6 Robert Haas : >> On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane wrote: >>> "David E. Wheeler" writes: >> Would it then be added as an ali

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Pavel Stehule
2011/10/6 Robert Haas : > On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane wrote: >> "David E. Wheeler" writes: > Would it then be added as an alias for := for named function parameters? > Or would that come still later? >> Once we do that, it will be impossible not merely deprecated to use

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane wrote: > "David E. Wheeler" writes: Would it then be added as an alias for := for named function parameters? Or would that come still later? > >>> Once we do that, it will be impossible not merely deprecated to use => >>> as an operator name.  I

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread David E. Wheeler
On Oct 6, 2011, at 10:46 AM, Tom Lane wrote: >> Okay. I kind of like := so there's no rush AFAIC. :-) > > Hmm ... actually, that raises another issue that I'm not sure whether > there's consensus for or not. Are we intending to keep name := value > syntax forever, as an alternative to the standa

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Tom Lane
"David E. Wheeler" writes: >>> Would it then be added as an alias for := for named function parameters? Or >>> would that come still later? >> Once we do that, it will be impossible not merely deprecated to use => >> as an operator name. I think that has to wait at least another release >> cycl

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread David E. Wheeler
On Oct 6, 2011, at 10:37 AM, Tom Lane wrote: >> Would it then be added as an alias for := for named function parameters? Or >> would that come still later? > > Once we do that, it will be impossible not merely deprecated to use => > as an operator name. I think that has to wait at least another

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Tom Lane
"David E. Wheeler" writes: > On Oct 6, 2011, at 9:46 AM, Tom Lane wrote: >>> +1. However, if that's the route we're traveling down, I think we had >>> better go ahead and remove the one remaining => operator from hstore >>> in 9.2: >> Fair enough. > Would it then be added as an alias for := for

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread David E . Wheeler
On Oct 6, 2011, at 9:46 AM, Tom Lane wrote: >> +1. However, if that's the route we're traveling down, I think we had >> better go ahead and remove the one remaining => operator from hstore >> in 9.2: > > Fair enough. Would it then be added as an alias for := for named function parameters? Or w

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Tom Lane
Robert Haas writes: > +1. However, if that's the route we're traveling down, I think we had > better go ahead and remove the one remaining => operator from hstore > in 9.2: Fair enough. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Robert Haas
On Thu, Oct 6, 2011 at 12:23 PM, Tom Lane wrote: > Royce Ausburn writes: >> Initial Review for patch: >> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php >> The patch adds a means of specifying named  cursor parameter arguments in >> pg/plsql. > >>       • Do we want that? > >>

Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-10-06 Thread Tom Lane
Royce Ausburn writes: > Initial Review for patch: > http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php > The patch adds a means of specifying named cursor parameter arguments in > pg/plsql. > • Do we want that? > I very rarely use pg/plsql, so I won't speak to its utilit