Re: [REVIEW] Patch for cursor calling with named parameters - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: [REVIEW] Patch for cursor calling with named parameters
Date
Msg-id 4EE77BB5.5040906@enterprisedb.com
Whole thread Raw
In response to Re: [REVIEW] Patch for cursor calling with named parameters  ("Kevin Grittner" <Kevin.Grittner@wicourts.gov>)
Responses Re: [REVIEW] Patch for cursor calling with named parameters  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Noah Misch
Date:
Subject: Re: foreign key locks, 2nd attempt
Next
From: Teodor Sigaev
Date:
Subject: Re: WIP: SP-GiST, Space-Partitioned GiST