Petr Jelinek escribió: > + <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. For > + example (with <varname>plpgsql.extra_warnings</> set to > + <varname>shadowed_variables</varname>): > +<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>
As a matter of style, I think the example should go after the <variablelist> is closed. Perhaps in the future, when we invent more extra warnings/errors, we might want to show more than one in a single example, for compactness. > +static bool > +plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource > source) > +{ > + if (strcmp(*newvalue, "all") == 0 || > + strcmp(*newvalue, "shadowed_variables") == 0 || > + strcmp(*newvalue, "none") == 0) > + return true; > + return false; > +} I'm not too clear on how this works when there is more than one possible value. > + DefineCustomStringVariable("plpgsql.extra_warnings", > + gettext_noop("List > of programming constructs which should produce a warning."), > + NULL, > + > &plpgsql_extra_warnings_list, > + "none", > + PGC_USERSET, 0, > + > plpgsql_extra_checks_check_hook, > + > plpgsql_extra_warnings_assign_hook, > + NULL); I think this should have the GUC_LIST_INPUT flag, and ensure that when multiple values are passed, we can process them all in a sane fashion. Other than this, the patch looks sane to me in a quick skim. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers