On 23/03/14 19:38, Pavel Stehule wrote:
doc should be enhanced by: <snip>
Docs updated. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index bddd458..2a9b327 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -4711,6 +4711,54 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$ </variablelist> </sect2> + <sect2 id="plpgsql-extra-checks"> + <title>Additional compile-time checks</title> + + <para> + To aid the user in finding instances of simple but common problems before + they cause harm, <application>PL/PgSQL</> provides additional + <replaceable>checks</>. When enabled, depending on the configuration, they + can be used to emit either a <literal>WARNING</> or an <literal>ERROR</> + during the compilation of a function. + </para> + + <para> + These additional checks are enabled through the configuration variables + <varname>plpgsql.extra_warnings</> for warnings and + <varname>plpgsql.extra_errors</> for errors. Both can be set either to + a comma-separated list of checks, <literal>"none"</> or <literal>"all"</>. + The default is <literal>"none"</>. Currently the list of available checks + includes only one: + <variablelist> + <varlistentry> + <term><varname>shadowed_variables</varname></term> + <listitem> + <para> + Checks if a declaration shadows a previously defined variable. + </para> + </listitem> + </varlistentry> + </variablelist> + + The following example shows the effect of <varname>plpgsql.extra_warnings</> + set to <varname>shadowed_variables</>: +<programlisting> +SET plpgsql.extra_warnings TO 'shadowed_variables'; + +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> + </sect2> </sect1> <!-- **** Porting from Oracle PL/SQL **** --> diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index 5afc2e5..12ac964 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 c0cb585..91186c6 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 f21393a..e659f8e 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 f3d1283..41fc940 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; diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 0405ef4..987c417 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -3208,6 +3208,79 @@ select footest(); 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.extra_warnings to 'shadowed_variables'; +-- 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 errors when shadowing a variable +set plpgsql.extra_errors to 'shadowed_variables'; +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.extra_errors; +reset plpgsql.extra_warnings; -- test scrollable cursor support create function sc_test() returns setof integer as $$ declare diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index d01011a..717d4fa 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -2689,6 +2689,68 @@ end$$ language plpgsql; select footest(); +-- test warnings when shadowing a variable + +set plpgsql.extra_warnings to 'shadowed_variables'; + +-- 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 errors when shadowing a variable + +set plpgsql.extra_errors to 'shadowed_variables'; + +create or replace function shadowtest(f1 int) + returns void as $$ +declare f1 int; begin end $$ language plpgsql; + +reset plpgsql.extra_errors; +reset plpgsql.extra_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