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

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 Ready for Committer.

This failed:

postgres=# do $$
declare
   foocur CURSOR ("insane /* arg" int4) IS SELECT generate_series(1, 
"insane /* arg");
begin
   OPEN foocur("insane /* arg" := 10);
end;
$$;
ERROR:  unterminated /* comment at or near "/* insane /* arg := */ 10;"
LINE 1: SELECT /* insane /* arg := */ 10;
                ^
QUERY:  SELECT /* insane /* arg := */ 10;
CONTEXT:  PL/pgSQL function "inline_code_block" line 5 at OPEN

I don't have much sympathy for anyone who uses argument names like that, 
but it nevertheless ought to not fail. A simple way to fix that is to 
constuct the query as: "value AS argname", instead of "/* argname := */ 
value". Then you can use the existing quote_identifier() function to do 
the necessary quoting.

I replaced the plpgsql_isidentassign() function with a more generic 
plpgsql_peek2() function, which allows you to peek ahead two tokens in 
the input stream, without eating them. It's implemented using the 
pushdown stack like plpgsql_isidentassign() was, but the division of 
labor between pl_scanner.c and gram.y seems more clear this way. I'm 
still not 100% happy with it. plpgsql_peek2() behaves differently from 
plpgsql_yylex(), in that it doesn't perform variable or unreserved 
keyword lookup. It could do that, but it would be quite pointless since 
the only current caller doesn't want variable or unreserved keyword 
lookup, so it would just have to work harder to undo them.

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.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com