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