From 05dbd4a7734e09bd1f835f4197d9befa1c00c4f3 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 4 Apr 2012 21:50:31 -0400 Subject: [PATCH] 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. --- src/pl/plpgsql/src/gram.y | 39 +++++++++++++++------------ src/test/regress/expected/plpgsql.out | 19 +++++++++++++ src/test/regress/sql/plpgsql.sql | 15 +++++++++++ 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y index 5a555afe89..c991765169 100644 --- 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 5455adef25..1e45919147 100644 --- 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 f577dc3cdc..2b60b678af 100644 --- 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 -- -- 2.40.0