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

On 2021/05/19 20:01, Bharath Rupireddy wrote:
> On Mon, May 17, 2021 at 4:23 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Tue, May 11, 2021 at 11:28 AM Bharath Rupireddy
>> <bharath.rupireddyforpostgres@gmail.com> wrote:
>>>
>>> On Mon, May 10, 2021 at 7:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>>
>>>> Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
>>>>> On Mon, May 10, 2021 at 12:00 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>>>> Yeah, this error message seems outright buggy.  However, it's a minor
>>>>>> matter.  Also, some people think "positive" is the same thing as
>>>>>> "non-negative", so maybe we need less ambiguous wording?
>>>>
>>>>> Since value 0 can't be considered as either a positive or negative
>>>>> integer, I think we can do as following(roughly):
>>>>
>>>>> if (value < 0) "requires a zero or positive integer value"
>>>>> if (value <= 0) "requires a positive integer value"
>>>>
>>>> I was thinking of avoiding the passive voice and writing
>>>>
>>>>          "foo must be greater than zero"
>>>
>>> +1 for "foo must be greater than zero" if (foo <= 0) kind of errors.
>>> But, we also have some values for which zero is accepted, see below
>>> error messages. How about the error message "foo must be greater than
>>> or equal to zero"?
>>>
>>
>> +1 for your proposed message for the cases where we have a check if
>> (foo < 0). Tom, Michael, do you see any problem with the proposed
>> message? We would like to make a similar change at another place [1]
>> so wanted to be consistent.
>>
>> [1] - https://www.postgresql.org/message-id/CALj2ACWGB9oHCR5ygkc8u6_QDqecObf9j2MxtOgsjZMMKsLj%3DQ%40mail.gmail.com
> 
> Thanks all for your inputs. PSA v2 patch that uses the new convention.

Thanks for the patch!

                  ereport(ERROR,
                          (errcode(ERRCODE_SYNTAX_ERROR),
-                         errmsg("%s requires a non-negative numeric value",
+                         errmsg("%s must be greater than or equal to zero",
                                  def->defname)));
          }
          else if (strcmp(def->defname, "extensions") == 0)
@@ -142,7 +142,7 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
              if (fetch_size <= 0)
                  ereport(ERROR,
                          (errcode(ERRCODE_SYNTAX_ERROR),
-                         errmsg("%s requires a non-negative integer value",
+                         errmsg("%s must be greater than zero",

I'm fine to convert "non-negative" word to "greater than" or "greater than
or equal to" in the messages. But this change also seems to get rid of
the information about the data type of the option from the message.
I'm not sure if this is an improvement. Probably isn't it better to
convert "requires a non-negative integer value" to "must be an integer value
greater than zero"?

Regards,

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