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.

diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 5a555afe89dec06b2af2457cf17e8dd1768029b5..c991765169a835466e2710126fe0f48553190af7 100644 (file)
--- a/src/pl/plpgsql/src/gram.y
+++ b/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)

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 5455adef25788462961fe5f4bf55a42e3fbca985..1e45919147f147e98c088254c415e08cb9f671fc 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/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
 --

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index f577dc3cdc679c0dd86004b4a4617bdbe8546b8d..2b60b678af33717531afbaa40c8af29fafaeaec8 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/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
 --