From 0e1275fb073cbbff2185f4067d67785e56941e50 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Wed, 28 Jul 2021 01:20:16 +0900 Subject: [PATCH] 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 --- contrib/postgres_fdw/option.c | 9 ++++++--- doc/src/sgml/sources.sgml | 10 ++++++++++ src/backend/partitioning/partbounds.c | 4 ++-- src/backend/utils/adt/tsquery_op.c | 2 +- src/test/modules/test_shm_mq/test.c | 10 +++++----- src/test/regress/expected/hash_part.out | 4 ++-- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c index 4593cbc540..c574ca2cf3 100644 --- a/contrib/postgres_fdw/option.c +++ b/contrib/postgres_fdw/option.c @@ -119,7 +119,10 @@ 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. + */ char *value; double real_val; bool is_parsed; @@ -136,7 +139,7 @@ postgres_fdw_validator(PG_FUNCTION_ARGS) if (real_val < 0) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("\"%s\" requires a non-negative floating point value", + errmsg("\"%s\" must be a floating point value greater than or equal to zero", def->defname))); } else if (strcmp(def->defname, "extensions") == 0) @@ -163,7 +166,7 @@ postgres_fdw_validator(PG_FUNCTION_ARGS) if (int_val <= 0) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("\"%s\" requires a non-negative integer value", + errmsg("\"%s\" must be an integer value greater than zero", def->defname))); } else if (strcmp(def->defname, "password_required") == 0) diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 3f2c40b750..e6ae02f2af 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -828,6 +828,16 @@ BETTER: unrecognized node type: 42 + + Non-negative + + Avoid non-negative as it is ambiguous + about whether it accepts zero. It's better to use + greater than zero or + greater than or equal to zero. + + + diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 12599da9a8..25018b1a8b 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -4760,11 +4760,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 0575b55272..8211e6c9bc 100644 --- a/src/backend/utils/adt/tsquery_op.c +++ b/src/backend/utils/adt/tsquery_op.c @@ -121,7 +121,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 e5e32abac2..2d8d695f97 100644 --- 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 b79c2b44c1..ac3aabee02 100644 --- 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