On 19/03/14 19:26, Alvaro Herrera wrote:
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.

Ok I can change that.


+static bool
+plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource 
source)

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.


Well, as we said with Marko in the original thread, the proper handling is left for whoever wants to add additional parameters, for the current implementation proper list handling is not really needed and it will only server to increase complexity of this simple patch quite late in the release cycle.

--
 Petr Jelinek                  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

Reply via email to