Re: Inaccurate error message when set fdw batch_size to 0 - Mailing list pgsql-hackers

On 2021/07/26 13:56, Bharath Rupireddy wrote:
> On Thu, Jul 15, 2021 at 7:54 PM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
>>
>> On Mon, Jul 12, 2021 at 10:11 PM Bharath Rupireddy
>> <bharath.rupireddyforpostgres@gmail.com> wrote:
>>>
>>> On Mon, Jul 12, 2021 at 9:20 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>>> +  <simplesect>
>>>> +   <title>Avoid Using <quote>non-negative</quote> Word in Error Messages</title>
>>>> +
>>>> +   <para>
>>>> +    Do not use <quote>non-negative</quote> word in error messages as it looks
>>>> +    ambiguous. Instead, use <quote>foo must be an integer value greater than zero</quote>
>>>> +    or  <quote>foo must be an integer value greater than or equal to zero</quote>
>>>> +    if option <quote>foo</quote> expects an integer value.
>>>> +   </para>
>>>> +  </simplesect>
>>>>
>>>> It seems suitable to put this guide under "Tricky Words to Avoid"
>>>> rather than adding it as separate section. Thought?
>>>
>>> +1. I will change.
>>
>> PSA v7 patch with the above change.
> 
> PSA v8 patch rebased on to latest master.

Thanks for updating the patch!

+  <formalpara>
+    <title>non-negative</title>
+   <para>
+    Do not use <quote>non-negative</quote> word in error messages as it looks
+    ambiguous. Instead, use <quote>foo must be an integer value greater than
+    zero</quote> or <quote>foo must be an integer value greater than or equal
+    to zero</quote> if option <quote>foo</quote> expects an integer value.
+   </para>
+  </formalpara>

This description looks a bit redundant. And IMO it's better to also document how "non-negative" is ambiguous. So what
aboutthe following description, instead? I replaced this description with the following. Patch attached. I also
uppercasedthe first character "n" of "non-negative" at the title for the sake of consistency with other items.
 

+  <formalpara>
+    <title>Non-negative</title>
+   <para>
+    Avoid <quote>non-negative</quote> as it is ambiguous
+    about whether it accepts zero.  It's better to use
+    <quote>greater than zero</quote> or
+    <quote>greater than or equal to zero</quote>.
+   </para>
+  </formalpara>


-    /* Number of workers should be non-negative. */
+    /* Number of parallel workers should be greater than zero. */
      Assert(nworkers >= 0);

This should be "greater than or equal to zero", instead? Anyway since this is comment not an error message, and also
thereare still other comments using "non-negative", I don't think we need to change only this comment for now. So I
excludedthis change from the patch. Maybe we can get rid of all "non-negative" from comments and documents later *if*
necessary.


-                 errmsg("repeat count size must be a non-negative integer")));
+                 errmsg("repeat count size must be greater than or equal to zero")));

-                 errmsg("number of workers must be a non-negative integer")));
+                 errmsg("number of workers must be greater than or equal to zero")));

Isn't it better to replace "be greater" with "be an integer value greater"? I applied this to the patch.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION