Skip to content

Commit 42e6b5c

Browse files
MasaoFujiiMasaoFujii
committed
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
1 parent c4ba87f commit 42e6b5c

File tree

6 files changed

+28
-15
lines changed

6 files changed

+28
-15
lines changed

‎contrib/postgres_fdw/option.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,18 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
116116
else if (strcmp(def->defname, "fdw_startup_cost") == 0 ||
117117
strcmp(def->defname, "fdw_tuple_cost") == 0)
118118
{
119-
/* these must have a non-negative numeric value */
119+
/*
120+
* These must have a floating point value greater than or equal to
121+
* zero.
122+
*/
120123
double val;
121124
char *endp;
122125

123126
val = strtod(defGetString(def), &endp);
124127
if (*endp || val < 0)
125128
ereport(ERROR,
126-
(errcode(ERRCODE_SYNTAX_ERROR),
127-
errmsg("%s requires a non-negative numeric value",
129+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
130+
errmsg("\"%s\" must be a floating point value greater than or equal to zero",
128131
def->defname)));
129132
}
130133
else if (strcmp(def->defname, "extensions") == 0)
@@ -139,8 +142,8 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
139142
fetch_size = strtol(defGetString(def), NULL, 10);
140143
if (fetch_size <= 0)
141144
ereport(ERROR,
142-
(errcode(ERRCODE_SYNTAX_ERROR),
143-
errmsg("%s requires a non-negative integer value",
145+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
146+
errmsg("\"%s\" must be an integer value greater than zero",
144147
def->defname)));
145148
}
146149
}

‎doc/src/sgml/sources.sgml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,16 @@ BETTER: unrecognized node type: 42
806806
</para>
807807
</formalpara>
808808

809+
<formalpara>
810+
<title>Non-negative</title>
811+
<para>
812+
Avoid <quote>non-negative</quote> as it is ambiguous
813+
about whether it accepts zero. It's better to use
814+
<quote>greater than zero</quote> or
815+
<quote>greater than or equal to zero</quote>.
816+
</para>
817+
</formalpara>
818+
809819
</simplesect>
810820

811821
<simplesect>

‎src/backend/partitioning/partbounds.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2083,11 +2083,11 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
20832083
if (modulus <= 0)
20842084
ereport(ERROR,
20852085
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2086-
errmsg("modulus for hash partition must be a positive integer")));
2086+
errmsg("modulus for hash partition must be an integer value greater than zero")));
20872087
if (remainder < 0)
20882088
ereport(ERROR,
20892089
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2090-
errmsg("remainder for hash partition must be a non-negative integer")));
2090+
errmsg("remainder for hash partition must be an integer value greater than or equal to zero")));
20912091
if (remainder >= modulus)
20922092
ereport(ERROR,
20932093
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),

‎src/backend/utils/adt/tsquery_op.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ tsquery_phrase_distance(PG_FUNCTION_ARGS)
120120
if (distance < 0 || distance > MAXENTRYPOS)
121121
ereport(ERROR,
122122
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
123-
errmsg("distance in phrase operator should be non-negative and less than %d",
123+
errmsg("distance in phrase operator must be an integer value between zero and %d inclusive",
124124
MAXENTRYPOS)));
125125
if (a->size == 0)
126126
{

‎src/test/modules/test_shm_mq/test.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,17 @@ test_shm_mq(PG_FUNCTION_ARGS)
5757
if (loop_count < 0)
5858
ereport(ERROR,
5959
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
60-
errmsg("repeat count size must be a non-negative integer")));
60+
errmsg("repeat count size must be an integer value greater than or equal to zero")));
6161

6262
/*
6363
* Since this test sends data using the blocking interfaces, it cannot
6464
* send data to itself. Therefore, a minimum of 1 worker is required. Of
6565
* course, a negative worker count is nonsensical.
6666
*/
67-
if (nworkers < 1)
67+
if (nworkers <= 0)
6868
ereport(ERROR,
6969
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
70-
errmsg("number of workers must be a positive integer")));
70+
errmsg("number of workers must be an integer value greater than zero")));
7171

7272
/* Set up dynamic shared memory segment and background workers. */
7373
test_shm_mq_setup(queue_size, nworkers, &seg, &outqh, &inqh);
@@ -149,7 +149,7 @@ test_shm_mq_pipelined(PG_FUNCTION_ARGS)
149149
if (loop_count < 0)
150150
ereport(ERROR,
151151
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
152-
errmsg("repeat count size must be a non-negative integer")));
152+
errmsg("repeat count size must be an integer value greater than or equal to zero")));
153153

154154
/*
155155
* Using the nonblocking interfaces, we can even send data to ourselves,
@@ -158,7 +158,7 @@ test_shm_mq_pipelined(PG_FUNCTION_ARGS)
158158
if (nworkers < 0)
159159
ereport(ERROR,
160160
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
161-
errmsg("number of workers must be a non-negative integer")));
161+
errmsg("number of workers must be an integer value greater than or equal to zero")));
162162

163163
/* Set up dynamic shared memory segment and background workers. */
164164
test_shm_mq_setup(queue_size, nworkers, &seg, &outqh, &inqh);

‎src/test/regress/expected/hash_part.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ SELECT satisfies_hash_partition('mchash1'::regclass, 4, 0, NULL);
1919
ERROR: "mchash1" is not a hash partitioned table
2020
-- invalid modulus
2121
SELECT satisfies_hash_partition('mchash'::regclass, 0, 0, NULL);
22-
ERROR: modulus for hash partition must be a positive integer
22+
ERROR: modulus for hash partition must be an integer value greater than zero
2323
-- remainder too small
2424
SELECT satisfies_hash_partition('mchash'::regclass, 1, -1, NULL);
25-
ERROR: remainder for hash partition must be a non-negative integer
25+
ERROR: remainder for hash partition must be an integer value greater than or equal to zero
2626
-- remainder too large
2727
SELECT satisfies_hash_partition('mchash'::regclass, 1, 1, NULL);
2828
ERROR: remainder for hash partition must be less than modulus

0 commit comments

Comments
 (0)