Support plpgsql variable names that conflict with unreserved SQL keywords.

A variable name matching a statement-introducing keyword, such as
"comment" or "update", caused parse failures if one tried to write
a statement using that keyword.  Commit bb1b8f69 already addressed
this scenario for the case of variable names matching unreserved
plpgsql keywords, but we didn't think about unreserved core-grammar
keywords.  The same heuristic (viz, it can't be a variable name
unless the next token is assignment or '[') should work fine for
that case too, and as a bonus the code gets shorter and less
duplicative.

Per bug #15555 from Feike Steenbergen.  Since this hasn't been
complained of before, and is easily worked around anyway,
I won't risk a back-patch.

Discussion: https://postgr.es/m/15555-149bbd70ddc7b4b6@postgresql.org

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 0e9ea105964b7dc15cc78e6c1c1d6867cfbef3b5..2454386af84500adf543250c09ed20b3cf3bc4be 100644 (file)
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -1353,6 +1353,9 @@ make_datum_param(PLpgSQL_expr *expr, int dno, int location)
  * yytxt is the original token text; we need this to check for quoting,
  * so that later checks for unreserved keywords work properly.
  *
+ * We attempt to recognize the token as a variable only if lookup is true
+ * and the plpgsql_IdentifierLookup context permits it.
+ *
  * If recognized as a variable, fill in *wdatum and return true;
  * if not recognized, fill in *word and return false.
  * (Note: those two pointers actually point to members of the same union,
@@ -1360,17 +1363,17 @@ make_datum_param(PLpgSQL_expr *expr, int dno, int location)
  * ----------
  */
 bool
-plpgsql_parse_word(char *word1, const char *yytxt,
+plpgsql_parse_word(char *word1, const char *yytxt, bool lookup,
                                   PLwdatum *wdatum, PLword *word)
 {
        PLpgSQL_nsitem *ns;
 
        /*
-        * We should do nothing in DECLARE sections.  In SQL expressions, there's
-        * no need to do anything either --- lookup will happen when the
-        * expression is compiled.
+        * We should not lookup variables in DECLARE sections.  In SQL
+        * expressions, there's no need to do so either --- lookup will happen
+        * when the expression is compiled.
         */
-       if (plpgsql_IdentifierLookup == IDENTIFIER_LOOKUP_NORMAL)
+       if (lookup && plpgsql_IdentifierLookup == IDENTIFIER_LOOKUP_NORMAL)
        {
                /*
                 * Do a lookup in the current namespace stack

diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
index 394d4658cd31ceb0787b1c0c9981fe12089bcfd4..8340628de3c8726c0d0e3e06ca8b4260d89b2814 100644 (file)
--- a/src/pl/plpgsql/src/pl_scanner.c
+++ b/src/pl/plpgsql/src/pl_scanner.c
@@ -328,6 +328,7 @@ plpgsql_yylex(void)
                                push_back_token(tok2, &aux2);
                                if (plpgsql_parse_word(aux1.lval.str,
                                                                           core_yy.scanbuf + aux1.lloc,
+                                                                          true,
                                                                           &aux1.lval.wdatum,
                                                                           &aux1.lval.word))
                                        tok1 = T_DATUM;
@@ -349,53 +350,40 @@ plpgsql_yylex(void)
                        push_back_token(tok2, &aux2);
 
                        /*
-                        * If we are at start of statement, prefer unreserved keywords
-                        * over variable names, unless the next token is assignment or
-                        * '[', in which case prefer variable names.  (Note we need not
-                        * consider '.' as the next token; that case was handled above,
-                        * and we always prefer variable names in that case.)  If we are
-                        * not at start of statement, always prefer variable names over
-                        * unreserved keywords.
+                        * See if it matches a variable name, except in the context where
+                        * we are at start of statement and the next token isn't
+                        * assignment or '['.  In that case, it couldn't validly be a
+                        * variable name, and skipping the lookup allows variable names to
+                        * be used that would conflict with plpgsql or core keywords that
+                        * introduce statements (e.g., "comment").  Without this special
+                        * logic, every statement-introducing keyword would effectively be
+                        * reserved in PL/pgSQL, which would be unpleasant.
+                        *
+                        * If it isn't a variable name, try to match against unreserved
+                        * plpgsql keywords.  If not one of those either, it's T_WORD.
+                        *
+                        * Note: we must call plpgsql_parse_word even if we don't want to
+                        * do variable lookup, because it sets up aux1.lval.word for the
+                        * non-variable cases.
                         */
-                       if (AT_STMT_START(plpgsql_yytoken) &&
-                               !(tok2 == '=' || tok2 == COLON_EQUALS || tok2 == '['))
+                       if (plpgsql_parse_word(aux1.lval.str,
+                                                                  core_yy.scanbuf + aux1.lloc,
+                                                                  (!AT_STMT_START(plpgsql_yytoken) ||
+                                                                       (tok2 == '=' || tok2 == COLON_EQUALS ||
+                                                                        tok2 == '[')),
+                                                                  &aux1.lval.wdatum,
+                                                                  &aux1.lval.word))
+                               tok1 = T_DATUM;
+                       else if (!aux1.lval.word.quoted &&
+                                        (kw = ScanKeywordLookup(aux1.lval.word.ident,
+                                                                                        unreserved_keywords,
+                                                                                        num_unreserved_keywords)))
                        {
-                               /* try for unreserved keyword, then for variable name */
-                               if (core_yy.scanbuf[aux1.lloc] != '"' &&
-                                       (kw = ScanKeywordLookup(aux1.lval.str,
-                                                                                       unreserved_keywords,
-                                                                                       num_unreserved_keywords)))
-                               {
-                                       aux1.lval.keyword = kw->name;
-                                       tok1 = kw->value;
-                               }
-                               else if (plpgsql_parse_word(aux1.lval.str,
-                                                                                       core_yy.scanbuf + aux1.lloc,
-                                                                                       &aux1.lval.wdatum,
-                                                                                       &aux1.lval.word))
-                                       tok1 = T_DATUM;
-                               else
-                                       tok1 = T_WORD;
+                               aux1.lval.keyword = kw->name;
+                               tok1 = kw->value;
                        }
                        else
-                       {
-                               /* try for variable name, then for unreserved keyword */
-                               if (plpgsql_parse_word(aux1.lval.str,
-                                                                          core_yy.scanbuf + aux1.lloc,
-                                                                          &aux1.lval.wdatum,
-                                                                          &aux1.lval.word))
-                                       tok1 = T_DATUM;
-                               else if (!aux1.lval.word.quoted &&
-                                                (kw = ScanKeywordLookup(aux1.lval.word.ident,
-                                                                                                unreserved_keywords,
-                                                                                                num_unreserved_keywords)))
-                               {
-                                       aux1.lval.keyword = kw->name;
-                                       tok1 = kw->value;
-                               }
-                               else
-                                       tok1 = T_WORD;
-                       }
+                               tok1 = T_WORD;
                }
        }
        else

diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 2dca49334af5ff4312a566a6f1c4afe4fb92bf4e..a7118b8386101ece1177248bf122b63b2e4f877d 100644 (file)
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1175,7 +1175,7 @@ extern PLpgSQL_function *plpgsql_compile(FunctionCallInfo fcinfo,
 extern PLpgSQL_function *plpgsql_compile_inline(char *proc_source);
 extern void plpgsql_parser_setup(struct ParseState *pstate,
                                         PLpgSQL_expr *expr);
-extern bool plpgsql_parse_word(char *word1, const char *yytxt,
+extern bool plpgsql_parse_word(char *word1, const char *yytxt, bool lookup,
                                   PLwdatum *wdatum, PLword *word);
 extern bool plpgsql_parse_dblword(char *word1, char *word2,
                                          PLwdatum *wdatum, PLcword *cword);

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index b9a987dbcfa983e0f75f1753c850433ae02be30a..d2f68e1e4cb1c59b6e30eacf972feb1332d14224 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -4781,6 +4781,27 @@ select unreserved_test();
               43
 (1 row)
 
+create or replace function unreserved_test() returns int as $$
+declare
+  comment int := 21;
+begin
+  comment := comment * 2;
+  comment on function unreserved_test() is 'this is a test';
+  return comment;
+end
+$$ language plpgsql;
+select unreserved_test();
+ unreserved_test 
+-----------------
+              42
+(1 row)
+
+select obj_description('unreserved_test()'::regprocedure, 'pg_proc');
+ obj_description 
+-----------------
+ this is a test
+(1 row)
+
 drop function unreserved_test();
 --
 -- Test FOREACH over arrays

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 01239e26bed0b40fb50389f3f78706b696973ca2..9c8cf752adbeef9a37d51b4a7e4dfa10163e43c8 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -3892,6 +3892,20 @@ $$ language plpgsql;
 
 select unreserved_test();
 
+create or replace function unreserved_test() returns int as $$
+declare
+  comment int := 21;
+begin
+  comment := comment * 2;
+  comment on function unreserved_test() is 'this is a test';
+  return comment;
+end
+$$ language plpgsql;
+
+select unreserved_test();
+
+select obj_description('unreserved_test()'::regprocedure, 'pg_proc');
+
 drop function unreserved_test();
 
 --