Avoid using ambiguous word "non-negative" in error messages.

The error messages using the word "non-negative" are confusing
because it's ambiguous about whether it accepts zero or not.
This commit improves those error messages by replacing it with
less ambiguous word like "greater than zero" or
"greater than or equal to zero".

Also this commit added the note about the word "non-negative" to
the error message style guide, to help writing the new error messages.

When postgres_fdw option fetch_size was set to zero, previously
the error message "fetch_size requires a non-negative integer value"
was reported. This error message was outright buggy. Therefore
back-patch to all supported versions where such buggy error message
could be thrown.

Reported-by: Hou Zhijie
Author: Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/OS0PR01MB5716415335A06B489F1B3A8194569@OS0PR01MB5716.jpnprd01.prod.outlook.com

diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 6854f1bd91e6831ef99ff6b6fc54a43983e59025..24d56173255cab7588ad9efa684e2d5acd4dea71 100644 (file)
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -116,15 +116,18 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
                else if (strcmp(def->defname, "fdw_startup_cost") == 0 ||
                                 strcmp(def->defname, "fdw_tuple_cost") == 0)
                {
-                       /* these must have a non-negative numeric value */
+                       /*
+                        * These must have a floating point value greater than or equal to
+                        * zero.
+                        */
                        double          val;
                        char       *endp;
 
                        val = strtod(defGetString(def), &endp);
                        if (*endp || val < 0)
                                ereport(ERROR,
-                                               (errcode(ERRCODE_SYNTAX_ERROR),
-                                                errmsg("%s requires a non-negative numeric value",
+                                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                                errmsg("\"%s\" must be a floating point value greater than or equal to zero",
                                                                def->defname)));
                }
                else if (strcmp(def->defname, "extensions") == 0)
@@ -139,8 +142,8 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
                        fetch_size = strtol(defGetString(def), NULL, 10);
                        if (fetch_size <= 0)
                                ereport(ERROR,
-                                               (errcode(ERRCODE_SYNTAX_ERROR),
-                                                errmsg("%s requires a non-negative integer value",
+                                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                                errmsg("\"%s\" must be an integer value greater than zero",
                                                                def->defname)));
                }
        }

diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml
index 21d2501095aaa529e039b998bda2ab6ff5d121bc..36fe2abceff9ef210ad7e103055640e11f768e85 100644 (file)
--- a/doc/src/sgml/sources.sgml
+++ b/doc/src/sgml/sources.sgml
@@ -806,6 +806,16 @@ BETTER: unrecognized node type: 42
    </para>
   </formalpara>
 
+  <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>
+
   </simplesect>
 
   <simplesect>

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 6103b96db64ce2c731db3473248e7778a2c195ba..457a4b4478a667bd31c32b9aedc201de94e8ccd9 100644 (file)
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -2083,11 +2083,11 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
        if (modulus <= 0)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("modulus for hash partition must be a positive integer")));
+                                errmsg("modulus for hash partition must be an integer value greater than zero")));
        if (remainder < 0)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("remainder for hash partition must be a non-negative integer")));
+                                errmsg("remainder for hash partition must be an integer value greater than or equal to zero")));
        if (remainder >= modulus)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),

diff --git a/src/backend/utils/adt/tsquery_op.c b/src/backend/utils/adt/tsquery_op.c
index 07bc609972d266c88c4ca2e4067d1c4bed5b4e3f..c14ce01d81b219a24df7f4c0dd540f437edd9cf7 100644 (file)
--- a/src/backend/utils/adt/tsquery_op.c
+++ b/src/backend/utils/adt/tsquery_op.c
@@ -120,7 +120,7 @@ tsquery_phrase_distance(PG_FUNCTION_ARGS)
        if (distance < 0 || distance > MAXENTRYPOS)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("distance in phrase operator should be non-negative and less than %d",
+                                errmsg("distance in phrase operator must be an integer value between zero and %d inclusive",
                                                MAXENTRYPOS)));
        if (a->size == 0)
        {

diff --git a/src/test/modules/test_shm_mq/test.c b/src/test/modules/test_shm_mq/test.c
index ebab98660179b1e0e30e9b52fb7aae6e13c3a3cd..f201ec1f0a5fa106458179aab647b7c4d42f5db3 100644 (file)
--- a/src/test/modules/test_shm_mq/test.c
+++ b/src/test/modules/test_shm_mq/test.c
@@ -57,17 +57,17 @@ test_shm_mq(PG_FUNCTION_ARGS)
        if (loop_count < 0)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("repeat count size must be a non-negative integer")));
+                                errmsg("repeat count size must be an integer value greater than or equal to zero")));
 
        /*
         * Since this test sends data using the blocking interfaces, it cannot
         * send data to itself.  Therefore, a minimum of 1 worker is required. Of
         * course, a negative worker count is nonsensical.
         */
-       if (nworkers < 1)
+       if (nworkers <= 0)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("number of workers must be a positive integer")));
+                                errmsg("number of workers must be an integer value greater than zero")));
 
        /* Set up dynamic shared memory segment and background workers. */
        test_shm_mq_setup(queue_size, nworkers, &seg, &outqh, &inqh);
@@ -149,7 +149,7 @@ test_shm_mq_pipelined(PG_FUNCTION_ARGS)
        if (loop_count < 0)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("repeat count size must be a non-negative integer")));
+                                errmsg("repeat count size must be an integer value greater than or equal to zero")));
 
        /*
         * Using the nonblocking interfaces, we can even send data to ourselves,
@@ -158,7 +158,7 @@ test_shm_mq_pipelined(PG_FUNCTION_ARGS)
        if (nworkers < 0)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("number of workers must be a non-negative integer")));
+                                errmsg("number of workers must be an integer value greater than or equal to zero")));
 
        /* Set up dynamic shared memory segment and background workers. */
        test_shm_mq_setup(queue_size, nworkers, &seg, &outqh, &inqh);

diff --git a/src/test/regress/expected/hash_part.out b/src/test/regress/expected/hash_part.out
index 8d4b337e62ad0dba8f8fe4c8ef1217d100c1198c..016d81a69c9a41d87dfcc092de552b35bd966d1a 100644 (file)
--- a/src/test/regress/expected/hash_part.out
+++ b/src/test/regress/expected/hash_part.out
@@ -19,10 +19,10 @@ SELECT satisfies_hash_partition('mchash1'::regclass, 4, 0, NULL);
 ERROR:  "mchash1" is not a hash partitioned table
 -- invalid modulus
 SELECT satisfies_hash_partition('mchash'::regclass, 0, 0, NULL);
-ERROR:  modulus for hash partition must be a positive integer
+ERROR:  modulus for hash partition must be an integer value greater than zero
 -- remainder too small
 SELECT satisfies_hash_partition('mchash'::regclass, 1, -1, NULL);
-ERROR:  remainder for hash partition must be a non-negative integer
+ERROR:  remainder for hash partition must be an integer value greater than or equal to zero
 -- remainder too large
 SELECT satisfies_hash_partition('mchash'::regclass, 1, 1, NULL);
 ERROR:  remainder for hash partition must be less than modulus