Extra warnings and errors for PL/pgSQL

Infrastructure to allow
 plpgsql.extra_warnings
 plpgsql.extra_errors

Initial extra checks only for shadowed_variables

Marko Tiikkaja and Petr Jelinek
Reviewed by Simon Riggs and Pavel Stěhule

4 files changed, 152 insertions, 0 deletions

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 5afc2e50044..12ac964d131 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -352,6 +352,9 @@ do_compile(FunctionCallInfo fcinfo,
 	function->out_param_varno = -1;		/* set up for no OUT param */
 	function->resolve_option = plpgsql_variable_conflict;
 	function->print_strict_params = plpgsql_print_strict_params;
+	/* only promote extra warnings and errors at CREATE FUNCTION time */
+	function->extra_warnings = forValidator ? plpgsql_extra_warnings : 0;
+	function->extra_errors = forValidator ? plpgsql_extra_errors : 0;
 
 	if (is_dml_trigger)
 		function->fn_is_trigger = PLPGSQL_DML_TRIGGER;
@@ -849,6 +852,9 @@ plpgsql_compile_inline(char *proc_source)
 	function->out_param_varno = -1;		/* set up for no OUT param */
 	function->resolve_option = plpgsql_variable_conflict;
 	function->print_strict_params = plpgsql_print_strict_params;
+	/* don't do extra validation for inline code as we don't want to add spam at runtime */
+	function->extra_warnings = 0;
+	function->extra_errors = 0;
 
 	plpgsql_ns_init();
 	plpgsql_ns_push(func_name);
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index c0cb58530be..91186c6b9ae 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -727,6 +727,21 @@ decl_varname	: T_WORD
 											  $1.ident, NULL, NULL,
 											  NULL) != NULL)
 							yyerror("duplicate declaration");
+
+						if (plpgsql_curr_compile->extra_warnings & PLPGSQL_XCHECK_SHADOWVAR ||
+							plpgsql_curr_compile->extra_errors & PLPGSQL_XCHECK_SHADOWVAR)
+						{
+							PLpgSQL_nsitem *nsi;
+							nsi = plpgsql_ns_lookup(plpgsql_ns_top(), false,
+													$1.ident, NULL, NULL, NULL);
+							if (nsi != NULL)
+								ereport(plpgsql_curr_compile->extra_errors & PLPGSQL_XCHECK_SHADOWVAR ? ERROR : WARNING,
+										(errcode(ERRCODE_DUPLICATE_ALIAS),
+										 errmsg("variable \"%s\" shadows a previously defined variable",
+												$1.ident),
+										 parser_errposition(@1)));
+						}
+
 					}
 				| unreserved_keyword
 					{
@@ -740,6 +755,21 @@ decl_varname	: T_WORD
 											  $1, NULL, NULL,
 											  NULL) != NULL)
 							yyerror("duplicate declaration");
+
+						if (plpgsql_curr_compile->extra_warnings & PLPGSQL_XCHECK_SHADOWVAR ||
+							plpgsql_curr_compile->extra_errors & PLPGSQL_XCHECK_SHADOWVAR)
+						{
+							PLpgSQL_nsitem *nsi;
+							nsi = plpgsql_ns_lookup(plpgsql_ns_top(), false,
+													$1, NULL, NULL, NULL);
+							if (nsi != NULL)
+								ereport(plpgsql_curr_compile->extra_errors & PLPGSQL_XCHECK_SHADOWVAR ? ERROR : WARNING,
+										(errcode(ERRCODE_DUPLICATE_ALIAS),
+										 errmsg("variable \"%s\" shadows a previously defined variable",
+												$1),
+										 parser_errposition(@1)));
+						}
+
 					}
 				;
 
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index f21393ae41d..e659f8e289a 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -25,6 +25,11 @@
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
+
+static bool plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource source);
+static void plpgsql_extra_warnings_assign_hook(const char *newvalue, void *extra);
+static void plpgsql_extra_errors_assign_hook(const char *newvalue, void *extra);
+
 PG_MODULE_MAGIC;
 
 /* Custom GUC variable */
@@ -39,10 +44,89 @@ int			plpgsql_variable_conflict = PLPGSQL_RESOLVE_ERROR;
 
 bool		plpgsql_print_strict_params = false;
 
+char		*plpgsql_extra_warnings_string = NULL;
+char		*plpgsql_extra_errors_string = NULL;
+int			plpgsql_extra_warnings;
+int			plpgsql_extra_errors;
+
 /* Hook for plugins */
 PLpgSQL_plugin **plugin_ptr = NULL;
 
 
+static bool
+plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource source)
+{
+	char	   *rawstring;
+	List	   *elemlist;
+	ListCell   *l;
+	int		    extrachecks = 0;
+	int		   *myextra;
+
+	if (pg_strcasecmp(*newvalue, "all") == 0)
+		extrachecks = PLPGSQL_XCHECK_ALL;
+	else if (pg_strcasecmp(*newvalue, "none") == 0)
+		extrachecks = PLPGSQL_XCHECK_NONE;
+	else
+	{
+		/* Need a modifiable copy of string */
+		rawstring = pstrdup(*newvalue);
+
+		/* Parse string into list of identifiers */
+		if (!SplitIdentifierString(rawstring, ',', &elemlist))
+		{
+			/* syntax error in list */
+			GUC_check_errdetail("List syntax is invalid.");
+			pfree(rawstring);
+			list_free(elemlist);
+			return false;
+		}
+
+		foreach(l, elemlist)
+		{
+			char	   *tok = (char *) lfirst(l);
+
+			if (pg_strcasecmp(tok, "shadowed_variables") == 0)
+				extrachecks |= PLPGSQL_XCHECK_SHADOWVAR;
+			else if (pg_strcasecmp(tok, "all") == 0 || pg_strcasecmp(tok, "none") == 0)
+			{
+				GUC_check_errdetail("Key word \"%s\" cannot be combined with other key words.", tok);
+				pfree(rawstring);
+				list_free(elemlist);
+				return false;
+			}
+			else
+			{
+				GUC_check_errdetail("Unrecognized key word: \"%s\".", tok);
+				pfree(rawstring);
+				list_free(elemlist);
+				return false;
+			}
+		}
+
+		pfree(rawstring);
+		list_free(elemlist);
+	}
+
+	myextra = (int *) malloc(sizeof(int));
+	*myextra = extrachecks;
+	*extra = (void *) myextra;
+
+	return true;
+}
+
+static void
+plpgsql_extra_warnings_assign_hook(const char *newvalue, void *extra)
+{
+	plpgsql_extra_warnings = *((int *) extra);
+}
+
+static void
+plpgsql_extra_errors_assign_hook(const char *newvalue, void *extra)
+{
+	plpgsql_extra_errors = *((int *) extra);
+}
+
+
 /*
  * _PG_init()			- library load-time initialization
  *
@@ -76,6 +160,26 @@ _PG_init(void)
 							 PGC_USERSET, 0,
 							 NULL, NULL, NULL);
 
+	DefineCustomStringVariable("plpgsql.extra_warnings",
+							   gettext_noop("List of programming constructs which should produce a warning."),
+							   NULL,
+							   &plpgsql_extra_warnings_string,
+							   "none",
+							   PGC_USERSET, GUC_LIST_INPUT,
+							   plpgsql_extra_checks_check_hook,
+							   plpgsql_extra_warnings_assign_hook,
+							   NULL);
+
+	DefineCustomStringVariable("plpgsql.extra_errors",
+							   gettext_noop("List of programming constructs which should produce an error."),
+							   NULL,
+							   &plpgsql_extra_errors_string,
+							   "none",
+							   PGC_USERSET, GUC_LIST_INPUT,
+							   plpgsql_extra_checks_check_hook,
+							   plpgsql_extra_errors_assign_hook,
+							   NULL);
+
 	EmitWarningsOnPlaceholders("plpgsql");
 
 	plpgsql_HashTableInit();
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index f3d1283015d..41fc9407a22 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -739,6 +739,10 @@ typedef struct PLpgSQL_function
 
 	bool		print_strict_params;
 
+	/* extra checks */
+	int 		extra_warnings;
+	int 		extra_errors;
+
 	int			ndatums;
 	PLpgSQL_datum **datums;
 	PLpgSQL_stmt_block *action;
@@ -881,6 +885,14 @@ extern int	plpgsql_variable_conflict;
 
 extern bool plpgsql_print_strict_params;
 
+/* extra compile-time checks */
+#define PLPGSQL_XCHECK_NONE			0
+#define PLPGSQL_XCHECK_SHADOWVAR	1
+#define PLPGSQL_XCHECK_ALL			((int) ~0)
+
+extern int plpgsql_extra_warnings;
+extern int plpgsql_extra_errors;
+
 extern bool plpgsql_check_syntax;
 extern bool plpgsql_DumpExecTree;