[CF 5722] v1 - Sanding down some edge cases for PL/pgSQL reserved words

This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/5722

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/[email protected]
Author(s): Tom Lane

Author: Commitfest Bot

src/pl/plpgsql/src/expected/plpgsql_misc.out

@@ -65,3 +65,25 @@ do $$ declare x public.foo%rowtype; begin end $$;
 ERROR:  relation "public.foo" does not exist
 CONTEXT:  compilation of PL/pgSQL function "inline_code_block" near line 1
 do $$ declare x public.misc_table%rowtype; begin end $$;
+-- Test handling of a reserved word as a record field name
+do $$ declare r record;
+begin
+  select 1 as x, 2 as foreach into r;
+  raise notice 'r.x = %', r.x;
+  raise notice 'r.foreach = %', r.foreach;
+end $$;
+NOTICE:  r.x = 1
+ERROR:  field name "foreach" is a reserved key word
+LINE 1: r.foreach
+        ^
+HINT:  Use double quotes to quote it.
+QUERY:  r.foreach
+CONTEXT:  PL/pgSQL function inline_code_block line 5 at RAISE
+do $$ declare r record;
+begin
+  select 1 as x, 2 as foreach into r;
+  raise notice 'r.x = %', r.x;
+  raise notice 'r."foreach" = %', r."foreach";
+end $$;
+NOTICE:  r.x = 1
+NOTICE:  r."foreach" = 2

src/pl/plpgsql/src/pl_comp.c

@@ -1201,17 +1201,22 @@ resolve_column_ref(ParseState *pstate, PLpgSQL_expr *expr,
 				}
 
 				/*
-				 * We should not get here, because a RECFIELD datum should
-				 * have been built at parse time for every possible qualified
-				 * reference to fields of this record.  But if we do, handle
-				 * it like field-not-found: throw error or return NULL.
+				 * Ideally we'd never get here, because a RECFIELD datum
+				 * should have been built at parse time for every qualified
+				 * reference to a field of this record that appears in the
+				 * source text.  However, plpgsql_yylex will not build such a
+				 * datum unless the field name lexes as token type IDENT.
+				 * Hence, if the would-be field name is a PL/pgSQL reserved
+				 * word, we lose.  Assume that that's what happened and tell
+				 * the user to quote it, unless the caller prefers we just
+				 * return NULL.
 				 */
 				if (error_if_no_field)
 					ereport(ERROR,
-							(errcode(ERRCODE_UNDEFINED_COLUMN),
-							 errmsg("record \"%s\" has no field \"%s\"",
-									(nnames_field == 1) ? name1 : name2,
+							(errcode(ERRCODE_SYNTAX_ERROR),
+							 errmsg("field name \"%s\" is a reserved key word",
 									colname),
+							 errhint("Use double quotes to quote it."),
 							 parser_errposition(pstate, cref->location)));
 			}
 			break;

src/pl/plpgsql/src/pl_gram.y

@@ -1368,7 +1368,8 @@ for_control		: for_variable K_IN
 						int			tok = yylex(&yylval, &yylloc, yyscanner);
 						int			tokloc = yylloc;
 
-						if (tok == K_EXECUTE)
+						if (tok_is_keyword(tok, &yylval,
+										   K_EXECUTE, "execute"))
 						{
 							/* EXECUTE means it's a dynamic FOR loop */
 							PLpgSQL_stmt_dynfors *new;
@@ -2135,7 +2136,8 @@ stmt_open		: K_OPEN cursor_variable
 								yyerror(&yylloc, NULL, yyscanner, "syntax error, expected \"FOR\"");
 
 							tok = yylex(&yylval, &yylloc, yyscanner);
-							if (tok == K_EXECUTE)
+							if (tok_is_keyword(tok, &yylval,
+											   K_EXECUTE, "execute"))
 							{
 								int			endtoken;
 
@@ -2536,6 +2538,7 @@ unreserved_keyword	:
 				| K_ERRCODE
 				| K_ERROR
 				| K_EXCEPTION
+				| K_EXECUTE
 				| K_EXIT
 				| K_FETCH
 				| K_FIRST
@@ -2581,6 +2584,7 @@ unreserved_keyword	:
 				| K_SLICE
 				| K_SQLSTATE
 				| K_STACKED
+				| K_STRICT
 				| K_TABLE
 				| K_TABLE_NAME
 				| K_TYPE
@@ -3514,7 +3518,8 @@ make_return_query_stmt(int location, YYSTYPE *yylvalp, YYLTYPE *yyllocp, yyscan_
 	new->stmtid = ++plpgsql_curr_compile->nstatements;
 
 	/* check for RETURN QUERY EXECUTE */
-	if ((tok = yylex(yylvalp, yyllocp, yyscanner)) != K_EXECUTE)
+	tok = yylex(yylvalp, yyllocp, yyscanner);
+	if (!tok_is_keyword(tok, yylvalp, K_EXECUTE, "execute"))
 	{
 		/* ordinary static query */
 		plpgsql_push_back_token(tok, yylvalp, yyllocp, yyscanner);
@@ -3597,7 +3602,7 @@ read_into_target(PLpgSQL_variable **target, bool *strict, YYSTYPE *yylvalp, YYLT
 		*strict = false;
 
 	tok = yylex(yylvalp, yyllocp, yyscanner);
-	if (strict && tok == K_STRICT)
+	if (strict && tok_is_keyword(tok, yylvalp, K_STRICT, "strict"))
 	{
 		*strict = true;
 		tok = yylex(yylvalp, yyllocp, yyscanner);

src/pl/plpgsql/src/pl_reserved_kwlist.h

@@ -33,7 +33,6 @@ PG_KEYWORD("case", K_CASE)
 PG_KEYWORD("declare", K_DECLARE)
 PG_KEYWORD("else", K_ELSE)
 PG_KEYWORD("end", K_END)
-PG_KEYWORD("execute", K_EXECUTE)
 PG_KEYWORD("for", K_FOR)
 PG_KEYWORD("foreach", K_FOREACH)
 PG_KEYWORD("from", K_FROM)
@@ -44,7 +43,6 @@ PG_KEYWORD("loop", K_LOOP)
 PG_KEYWORD("not", K_NOT)
 PG_KEYWORD("null", K_NULL)
 PG_KEYWORD("or", K_OR)
-PG_KEYWORD("strict", K_STRICT)
 PG_KEYWORD("then", K_THEN)
 PG_KEYWORD("to", K_TO)
 PG_KEYWORD("using", K_USING)

src/pl/plpgsql/src/pl_scanner.c

@@ -53,7 +53,7 @@ IdentifierLookup plpgsql_IdentifierLookup = IDENTIFIER_LOOKUP_NORMAL;
  * We try to avoid reserving more keywords than we have to; but there's
  * little point in not reserving a word if it's reserved in the core grammar.
  * Currently, the following words are reserved here but not in the core:
- * BEGIN BY DECLARE EXECUTE FOREACH IF LOOP STRICT WHILE
+ * BEGIN BY DECLARE FOREACH IF LOOP WHILE
  */
 
 /* ScanKeywordList lookup data for PL/pgSQL keywords */

src/pl/plpgsql/src/pl_unreserved_kwlist.h

@@ -58,6 +58,7 @@ PG_KEYWORD("elsif", K_ELSIF)
 PG_KEYWORD("errcode", K_ERRCODE)
 PG_KEYWORD("error", K_ERROR)
 PG_KEYWORD("exception", K_EXCEPTION)
+PG_KEYWORD("execute", K_EXECUTE)
 PG_KEYWORD("exit", K_EXIT)
 PG_KEYWORD("fetch", K_FETCH)
 PG_KEYWORD("first", K_FIRST)
@@ -103,6 +104,7 @@ PG_KEYWORD("scroll", K_SCROLL)
 PG_KEYWORD("slice", K_SLICE)
 PG_KEYWORD("sqlstate", K_SQLSTATE)
 PG_KEYWORD("stacked", K_STACKED)
+PG_KEYWORD("strict", K_STRICT)
 PG_KEYWORD("table", K_TABLE)
 PG_KEYWORD("table_name", K_TABLE_NAME)
 PG_KEYWORD("type", K_TYPE)

src/pl/plpgsql/src/sql/plpgsql_misc.sql

@@ -37,3 +37,19 @@ do $$ declare x foo.bar%rowtype; begin end $$;
 do $$ declare x foo.bar.baz%rowtype; begin end $$;
 do $$ declare x public.foo%rowtype; begin end $$;
 do $$ declare x public.misc_table%rowtype; begin end $$;
+
+-- Test handling of a reserved word as a record field name
+
+do $$ declare r record;
+begin
+  select 1 as x, 2 as foreach into r;
+  raise notice 'r.x = %', r.x;
+  raise notice 'r.foreach = %', r.foreach;
+end $$;
+
+do $$ declare r record;
+begin
+  select 1 as x, 2 as foreach into r;
+  raise notice 'r.x = %', r.x;
+  raise notice 'r."foreach" = %', r."foreach";
+end $$;