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
 --