Re: [HACKERS] plpgsql - additional extra checks - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: [HACKERS] plpgsql - additional extra checks
Date
Msg-id CAFj8pRBFHUHxRz=aYt_L7L+8gqyOJT3SAkpath8SvZCQ0uj2NA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] plpgsql - additional extra checks  ("Tels" <nospam-abuse@bloodgate.com>)
Responses Re: [HACKERS] plpgsql - additional extra checks  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers


2018-03-20 13:56 GMT+01:00 Tels <nospam-abuse@bloodgate.com>:
Hello Pavel and Tomas,

On Tue, March 20, 2018 12:36 am, Pavel Stehule wrote:
> 2018-03-19 21:47 GMT+01:00 Tomas Vondra <tomas.vondra@2ndquadrant.com>:
>
>> Hi,
>>
>> I'm looking at the updated patch (plpgsql-extra-check-180316.patch), and
>> this time it applies and builds OK. The one thing I noticed is that the
>> documentation still uses the old wording for strict_multi_assignement:
>>
>> WARNING:  Number of evaluated fields does not match expected.
>> HINT:  strict_multi_assignement check of extra_warnings is active.
>> WARNING:  Number of evaluated fields does not match expected.
>> HINT:  strict_multi_assignement check of extra_warnings is active.
>>
>> This was reworded to "Number of source and target fields in assignment
>> does not match."
>>

I believe the correct wording should be:

"Number of source and target fields in assignment do not match."

ecause comparing one number to the other means "the number A and B _do_
not match", not "the number A does not match number B".

Also there is an inconsistent quoting here:

+   <para>
+    Setting <varname>plpgsql.extra_warnings</varname>, or
+    <varname>plpgsql.extra_errors</varname>, as appropriate, to
<literal>all</literal>

no quotes for 'all'.

+    is encouraged in development and/or testing environments.
+   </para>
+
+   <para>
+    These additional checks are enabled through the configuration variables
+    <varname>plpgsql.extra_warnings</varname> for warnings and
+    <varname>plpgsql.extra_errors</varname> for errors. Both can be set
either to
+    a comma-separated list of checks, <literal>"none"</literal> or
+    <literal>"all"</literal>.

quotes here around '"all"'. I think it should be one or the other in both
cases.

Also:

+ Currently
+    the list of available checks includes only one:

but then it lists more than one check?

all mentioned issues should be fixed

Thank you for your time :)

Regards

Pavel
 

Best wishes,

Tels

Attachment

pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: pgsql: Fix CommandCounterIncrement in partition-related DDL
Next
From: Tom Lane
Date:
Subject: Re: configure's checks for --enable-tap-tests are insufficient