Hi everyone,
Here's a new version of the patch. Added new tests and docs and changed
the GUCs per discussion.
plpgsql.warnings_as_errors only affects compilation at CREATE FUNCTION time:
=# set plpgsql.warnings to 'all';
SET
=#* set plpgsql.warnings_as_errors to true;
SET
=#* select foof(1); -- compiled since it's the first call in this session
WARNING: variable "f1" shadows a previously defined variable
LINE 2: declare f1 int;
^
foof
------
(1 row)
=#* create or replace function foof(f1 int) returns void as
$$
declare f1 int;
begin
end $$ language plpgsql;
ERROR: variable "f1" shadows a previously defined variable
LINE 3: declare f1 int;
Currently, plpgsql_warnings is a boolean since there's only one warning
we implement. The idea is to make it a bit field of some kind in the
future when we add more warnings. Starting that work for 9.4 seemed
like overkill, though. I tried to keep things simple.
Regards,
Marko Tiikkaja
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 4712,4717 **** a_output := a_output || $$ if v_$$ || referrer_keys.kind ||
$$ like '$$
--- 4712,4762 ----
</variablelist>
</sect2>
+ <sect2 id="plpgsql-warnings">
+ <title>Warnings</title>
+
+ <para>
+ To aid the user in finding instances of simple but common problems before
+ they cause harm, <application>PL/PgSQL</> provides a number of
+ <replaceable>warnings</>. When enabled, a <literal>WARNING</> is emitted
+ during the compilation of a function. Optionally, if
+ <varname>plpgsql.warnings_as_errors</> is set, an <literal>ERROR</> is
+ raised when attempting to create a function which would otherwise result
in
+ a warning.
+ </para>
+
+ <para>
+ Warnings are enabled through the configuration variable
+ <varname>plpgsql.warnings</>. It can be set either to a comma-separated
list
+ of warnings, <literal>"none"</> or <literal>"all"</>. The default is
+ <literal>"none"</>. Currently the list of available warnings includes only
+ one:
+ <variablelist>
+ <varlistentry>
+ <term><varname>shadow</varname></term>
+ <listitem>
+ <para>
+ Warns when a declaration shadows a previously defined variable. For
+ example:
+ <programlisting>
+ CREATE FUNCTION foo(f1 int) RETURNS int AS $$
+ DECLARE
+ f1 int;
+ BEGIN
+ RETURN f1;
+ END
+ $$ LANGUAGE plpgsql;
+ WARNING: variable "f1" shadows a previously defined variable
+ LINE 3: f1 int;
+ ^
+ CREATE FUNCTION
+ </programlisting>
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+ </sect2>
</sect1>
<!-- **** Porting from Oracle PL/SQL **** -->
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***************
*** 352,357 **** do_compile(FunctionCallInfo fcinfo,
--- 352,360 ----
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;
+ function->warnings = plpgsql_warnings;
+ /* only promote warnings to errors at CREATE FUNCTION time */
+ function->warnings_as_errors = plpgsql_warnings_as_errors &&
forValidator;
if (is_dml_trigger)
function->fn_is_trigger = PLPGSQL_DML_TRIGGER;
***************
*** 849,854 **** plpgsql_compile_inline(char *proc_source)
--- 852,860 ----
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;
+ function->warnings = plpgsql_warnings;
+ /* never promote warnings to errors */
+ function->warnings_as_errors = false;
plpgsql_ns_init();
plpgsql_ns_push(func_name);
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
***************
*** 727,732 **** decl_varname : T_WORD
--- 727,746 ----
$1.ident, NULL, NULL,
NULL) != NULL)
yyerror("duplicate
declaration");
+
+ if
(plpgsql_curr_compile->warnings)
+ {
+ PLpgSQL_nsitem *nsi;
+ nsi =
plpgsql_ns_lookup(plpgsql_ns_top(), false,
+
$1.ident, NULL, NULL, NULL);
+ if (nsi != NULL)
+
ereport(plpgsql_curr_compile->warnings_as_errors ? ERROR : WARNING,
+
(errcode(ERRCODE_DUPLICATE_ALIAS),
+
errmsg("variable \"%s\" shadows a previously defined variable",
+
$1.ident),
+
parser_errposition(@1)));
+ }
+
}
| unreserved_keyword
{
***************
*** 740,745 **** decl_varname : T_WORD
--- 754,773 ----
$1, NULL, NULL,
NULL) != NULL)
yyerror("duplicate
declaration");
+
+ if
(plpgsql_curr_compile->warnings)
+ {
+ PLpgSQL_nsitem *nsi;
+ nsi =
plpgsql_ns_lookup(plpgsql_ns_top(), false,
+
$1, NULL, NULL, NULL);
+ if (nsi != NULL)
+
ereport(plpgsql_curr_compile->warnings_as_errors ? ERROR : WARNING,
+
(errcode(ERRCODE_DUPLICATE_ALIAS),
+
errmsg("variable \"%s\" shadows a previously defined variable",
+
$1),
+
parser_errposition(@1)));
+ }
+
}
;
*** a/src/pl/plpgsql/src/pl_handler.c
--- b/src/pl/plpgsql/src/pl_handler.c
***************
*** 25,30 ****
--- 25,34 ----
#include "utils/lsyscache.h"
#include "utils/syscache.h"
+
+ static bool plpgsql_warnings_check_hook(char **newvalue, void **extra,
GucSource source);
+ static void plpgsql_warnings_assign_hook(const char *newvalue, void *extra);
+
PG_MODULE_MAGIC;
/* Custom GUC variable */
***************
*** 39,48 **** int plpgsql_variable_conflict =
PLPGSQL_RESOLVE_ERROR;
--- 43,76 ----
bool plpgsql_print_strict_params = false;
+ char *plpgsql_warnings_list = NULL;
+ bool plpgsql_warnings;
+ bool plpgsql_warnings_as_errors;
+
/* Hook for plugins */
PLpgSQL_plugin **plugin_ptr = NULL;
+ static bool
+ plpgsql_warnings_check_hook(char **newvalue, void **extra, GucSource source)
+ {
+ if (strcmp(*newvalue, "all") == 0 ||
+ strcmp(*newvalue, "shadow") == 0 ||
+ strcmp(*newvalue, "none") == 0)
+ return true;
+ return false;
+ }
+
+ static void
+ plpgsql_warnings_assign_hook(const char *newvalue, void *extra)
+ {
+ if (strcmp(newvalue, "all") == 0 ||
+ strcmp(newvalue, "shadow") == 0)
+ plpgsql_warnings = true;
+ else
+ plpgsql_warnings = false;
+ }
+
/*
* _PG_init() - library load-time initialization
*
***************
*** 76,81 **** _PG_init(void)
--- 104,127 ----
PGC_USERSET, 0,
NULL, NULL, NULL);
+ DefineCustomStringVariable("plpgsql.warnings",
+ gettext_noop("List
of programming constructs which should produce a warning."),
+ NULL,
+
&plpgsql_warnings_list,
+ "none",
+ PGC_USERSET, 0,
+
plpgsql_warnings_check_hook,
+
plpgsql_warnings_assign_hook,
+ NULL);
+
+ DefineCustomBoolVariable("plpgsql.warnings_as_errors",
+ gettext_noop("If set,
attempting to compile a function which would emit a warning will raise an error
instead."),
+ NULL,
+
&plpgsql_warnings_as_errors,
+ false,
+ PGC_USERSET, 0,
+ NULL, NULL, NULL);
+
EmitWarningsOnPlaceholders("plpgsql");
plpgsql_HashTableInit();
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 739,744 **** typedef struct PLpgSQL_function
--- 739,748 ----
bool print_strict_params;
+ /* warnings */
+ bool warnings;
+ bool warnings_as_errors;
+
int ndatums;
PLpgSQL_datum **datums;
PLpgSQL_stmt_block *action;
***************
*** 881,886 **** extern int plpgsql_variable_conflict;
--- 885,893 ----
extern bool plpgsql_print_strict_params;
+ extern bool plpgsql_warnings;
+ extern bool plpgsql_warnings_as_errors;
+
extern bool plpgsql_check_syntax;
extern bool plpgsql_DumpExecTree;
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 3208,3213 **** select footest();
--- 3208,3286 ----
ERROR: query returned more than one row
DETAIL: parameters: p1 = '2', p3 = 'foo'
CONTEXT: PL/pgSQL function footest() line 10 at SQL statement
+ -- test warnings when shadowing a variable
+ set plpgsql.warnings to 'shadow';
+ -- simple shadowing of input and output parameters
+ create or replace function shadowtest(in1 int)
+ returns table (out1 int) as $$
+ declare
+ in1 int;
+ out1 int;
+ begin
+ end
+ $$ language plpgsql;
+ WARNING: variable "in1" shadows a previously defined variable
+ LINE 4: in1 int;
+ ^
+ WARNING: variable "out1" shadows a previously defined variable
+ LINE 5: out1 int;
+ ^
+ drop function shadowtest(int);
+ -- shadowing in a second DECLARE block
+ create or replace function shadowtest()
+ returns void as $$
+ declare
+ f1 int;
+ begin
+ declare
+ f1 int;
+ begin
+ end;
+ end$$ language plpgsql;
+ WARNING: variable "f1" shadows a previously defined variable
+ LINE 7: f1 int;
+ ^
+ drop function shadowtest();
+ -- several levels of shadowing
+ create or replace function shadowtest(in1 int)
+ returns void as $$
+ declare
+ in1 int;
+ begin
+ declare
+ in1 int;
+ begin
+ end;
+ end$$ language plpgsql;
+ WARNING: variable "in1" shadows a previously defined variable
+ LINE 4: in1 int;
+ ^
+ WARNING: variable "in1" shadows a previously defined variable
+ LINE 7: in1 int;
+ ^
+ drop function shadowtest(int);
+ -- shadowing in cursor definitions
+ create or replace function shadowtest()
+ returns void as $$
+ declare
+ f1 int;
+ c1 cursor (f1 int) for select 1;
+ begin
+ end$$ language plpgsql;
+ WARNING: variable "f1" shadows a previously defined variable
+ LINE 5: c1 cursor (f1 int) for select 1;
+ ^
+ drop function shadowtest();
+ -- test warnings_as_errors
+ set plpgsql.warnings_as_errors to 'on';
+ create or replace function shadowtest(f1 int)
+ returns void as $$
+ declare f1 int; begin end $$ language plpgsql;
+ ERROR: variable "f1" shadows a previously defined variable
+ LINE 3: declare f1 int; begin end $$ language plpgsql;
+ ^
+ reset plpgsql.warnings_as_errors;
+ reset plpgsql.warnings;
-- test scrollable cursor support
create function sc_test() returns setof integer as $$
declare
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
***************
*** 2689,2694 **** end$$ language plpgsql;
--- 2689,2756 ----
select footest();
+ -- test warnings when shadowing a variable
+
+ set plpgsql.warnings to 'shadow';
+
+ -- simple shadowing of input and output parameters
+ create or replace function shadowtest(in1 int)
+ returns table (out1 int) as $$
+ declare
+ in1 int;
+ out1 int;
+ begin
+ end
+ $$ language plpgsql;
+ drop function shadowtest(int);
+
+ -- shadowing in a second DECLARE block
+ create or replace function shadowtest()
+ returns void as $$
+ declare
+ f1 int;
+ begin
+ declare
+ f1 int;
+ begin
+ end;
+ end$$ language plpgsql;
+ drop function shadowtest();
+
+ -- several levels of shadowing
+ create or replace function shadowtest(in1 int)
+ returns void as $$
+ declare
+ in1 int;
+ begin
+ declare
+ in1 int;
+ begin
+ end;
+ end$$ language plpgsql;
+ drop function shadowtest(int);
+
+ -- shadowing in cursor definitions
+ create or replace function shadowtest()
+ returns void as $$
+ declare
+ f1 int;
+ c1 cursor (f1 int) for select 1;
+ begin
+ end$$ language plpgsql;
+ drop function shadowtest();
+
+ -- test warnings_as_errors
+
+ set plpgsql.warnings_as_errors to 'on';
+
+ create or replace function shadowtest(f1 int)
+ returns void as $$
+ declare f1 int; begin end $$ language plpgsql;
+
+ reset plpgsql.warnings_as_errors;
+ reset plpgsql.warnings;
+
-- test scrollable cursor support
create function sc_test() returns setof integer as $$
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
