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 Linnakangasheikki.linnakan...@enterprisedb.com 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

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 Linnakangasheikki.linnakan...@enterprisedb.com 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

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: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-13 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com 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

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 Grittnerkgri...@wicourts.gov wrote: Yeb Havingayebhavi...@gmail.com 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

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-11 Thread Yeb Havinga
On 2011-12-06 17:58, Kevin Grittner wrote: Kevin Grittnerkgri...@wicourts.gov wrote: Yeb Havingayebhavi...@gmail.com 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

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 Grittnerkgri...@wicourts.gov 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

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

2011-12-06 Thread Kevin Grittner
Kevin Grittner kgri...@wicourts.gov wrote: Yeb Havinga yebhavi...@gmail.com 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

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,

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

2011-12-05 Thread Kevin Grittner
Yeb Havinga yebhavi...@gmail.com 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)

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

2011-12-05 Thread Kevin Grittner
Kevin Grittner kevin.gritt...@wicourts.gov wrote: Yeb Havinga yebhavi...@gmail.com 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

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 version

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 Havingayebhavi...@gmail.com 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

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 Havingayebhavi...@gmail.com 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

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 Havingayebhavi...@gmail.com 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 -

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

2011-10-14 Thread Tom Lane
Yeb Havinga yebhavi...@gmail.com 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

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 was a

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

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 the

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

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-06 Thread Tom Lane
Royce Ausburn royce...@inomial.com 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

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 t...@sss.pgh.pa.us wrote: Royce Ausburn royce...@inomial.com 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.    

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

2011-10-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com 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

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 would

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

2011-10-06 Thread Tom Lane
David E. Wheeler da...@kineticode.com 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

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 da...@kineticode.com 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

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 standard

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 t...@sss.pgh.pa.us wrote: David E. Wheeler da...@kineticode.com 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

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

2011-10-06 Thread Pavel Stehule
2011/10/6 Robert Haas robertmh...@gmail.com: On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: David E. Wheeler da...@kineticode.com 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

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 robertmh...@gmail.com: On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: David E. Wheeler