Fix plpgsql named-cursor-parameter feature for variable name conflicts.

The parser got confused if a cursor parameter had the same name as
a plpgsql variable.  Reported and diagnosed by Yeb Havinga, though
this isn't exactly his proposed fix.

Also, some mostly-but-not-entirely-cosmetic adjustments to the original
named-cursor-parameter patch, for code readability and better error
diagnostics.

Author: tglsfdc

src/pl/plpgsql/src/gram.y

@@ -3394,11 +3394,11 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 	PLpgSQL_expr *expr;
 	PLpgSQL_row *row;
 	int			tok;
-	int			argc = 0;
+	int			argc;
 	char	  **argv;
 	StringInfoData ds;
 	char	   *sqlstart = "SELECT ";
-	bool		named = false;
+	bool		any_named = false;
 
 	tok = yylex();
 	if (cursor->cursor_explicit_argrow < 0)
@@ -3417,9 +3417,6 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 		return NULL;
 	}
 
-	row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
-	argv = (char **) palloc0(row->nfields * sizeof(char *));
-
 	/* Else better provide arguments */
 	if (tok != '(')
 		ereport(ERROR,
@@ -3431,6 +3428,9 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 	/*
 	 * Read the arguments, one by one.
 	 */
+	row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+	argv = (char **) palloc0(row->nfields * sizeof(char *));
+
 	for (argc = 0; argc < row->nfields; argc++)
 	{
 		PLpgSQL_expr *item;
@@ -3445,11 +3445,16 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 		if (tok1 == IDENT && tok2 == COLON_EQUALS)
 		{
 			char   *argname;
+			IdentifierLookup save_IdentifierLookup;
 
-			/* Read the argument name, and find its position */
+			/* Read the argument name, ignoring any matching variable */
+			save_IdentifierLookup = plpgsql_IdentifierLookup;
+			plpgsql_IdentifierLookup = IDENTIFIER_LOOKUP_DECLARE;
 			yylex();
 			argname = yylval.str;
+			plpgsql_IdentifierLookup = save_IdentifierLookup;
 
+			/* Match argument name to cursor arguments */
 			for (argpos = 0; argpos < row->nfields; argpos++)
 			{
 				if (strcmp(row->fieldnames[argpos], argname) == 0)
@@ -3470,11 +3475,18 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 			if (tok2 != COLON_EQUALS)
 				yyerror("syntax error");
 
-			named = true;
+			any_named = true;
 		}
 		else
 			argpos = argc;
 
+		if (argv[argpos] != NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("duplicate value for cursor \"%s\" parameter \"%s\"",
+							cursor->refname, row->fieldnames[argpos]),
+					 parser_errposition(arglocation)));
+
 		/*
 		 * Read the value expression. To provide the user with meaningful
 		 * parse error positions, we check the syntax immediately, instead of
@@ -3491,6 +3503,8 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 								  false, /* do not trim */
 								  NULL, &endtoken);
 
+		argv[argpos] = item->query + strlen(sqlstart);
+
 		if (endtoken == ')' && !(argc == row->nfields - 1))
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -3504,15 +3518,6 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 					 errmsg("too many arguments for cursor \"%s\"",
 							cursor->refname),
 					 parser_errposition(yylloc)));
-
-		if (argv[argpos] != NULL)
-			ereport(ERROR,
-					(errcode(ERRCODE_SYNTAX_ERROR),
-					 errmsg("duplicate value for cursor \"%s\" parameter \"%s\"",
-							cursor->refname, row->fieldnames[argpos]),
-					 parser_errposition(arglocation)));
-
-		argv[argpos] = item->query + strlen(sqlstart);
 	}
 
 	/* Make positional argument list */
@@ -3527,7 +3532,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 		 * the parameter name for meaningful runtime errors.
 		 */
 		appendStringInfoString(&ds, argv[argc]);
-		if (named)
+		if (any_named)
 			appendStringInfo(&ds, " AS %s",
 							 quote_identifier(row->fieldnames[argc]));
 		if (argc < row->nfields - 1)

src/test/regress/expected/plpgsql.out

@@ -2420,6 +2420,25 @@ select namedparmcursor_test8();
                      0
 (1 row)
 
+-- cursor parameter name can match plpgsql variable or unreserved keyword
+create function namedparmcursor_test9(p1 int) returns int4 as $$
+declare
+  c1 cursor (p1 int, p2 int, debug int) for
+    select count(*) from tenk1 where thousand = p1 and tenthous = p2
+      and four = debug;
+  p2 int4 := 1006;
+  n int4;
+begin
+  open c1 (p1 := p1, p2 := p2, debug := 2);
+  fetch c1 into n;
+  return n;
+end $$ language plpgsql;
+select namedparmcursor_test9(6);
+ namedparmcursor_test9 
+-----------------------
+                     1
+(1 row)
+
 --
 -- tests for "raise" processing
 --

src/test/regress/sql/plpgsql.sql

@@ -2053,6 +2053,21 @@ begin
 end $$ language plpgsql;
 select namedparmcursor_test8();
 
+-- cursor parameter name can match plpgsql variable or unreserved keyword
+create function namedparmcursor_test9(p1 int) returns int4 as $$
+declare
+  c1 cursor (p1 int, p2 int, debug int) for
+    select count(*) from tenk1 where thousand = p1 and tenthous = p2
+      and four = debug;
+  p2 int4 := 1006;
+  n int4;
+begin
+  open c1 (p1 := p1, p2 := p2, debug := 2);
+  fetch c1 into n;
+  return n;
+end $$ language plpgsql;
+select namedparmcursor_test9(6);
+
 --
 -- tests for "raise" processing
 --