On 2025/06/27 13:09, Yugo Nagata wrote:
On Fri, 27 Jun 2025 12:22:17 +0900
Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
However, your patch is clear, and it seems we don't need to worry about this
concern.
Your patch looks good to me.
Thanks for reviewing the patch! I've marked it as ready for committer.
I have a few minor comment on the patch.
+ if (ival < 0)
+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("%s requires a
Boolean value, a non-negative "
+ "integer, or the
string \"match\"",
+
def->defname)));
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("%s requires a Boolean value or \"match\"",
+ errmsg("%s requires a Boolean value, a non-negative
integer, "
+ "or the string \"match\"",
def->defname)));
These two pieces of code raise the same error, but with different error codes:
one returns ERRCODE_INVALID_PARAMETER_VALUE, while the other returns
ERRCODE_SYNTAX_ERROR.
I believe the former is more appropriate, although the existing code uses the
latter. In any case, the error codes should be consistent.
I'm not sure there's an actual rule like "the error code must match
if the error message is the same." But if using the same message with
a different error code is confusing, I'm fine with changing
the earlier message. For example:
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("%s requires a
Boolean value, a non-negative "
- "integer, or the
string \"match\"",
+ errmsg("a negative integer
value cannot be "
+ "specified
for %s",
def->defname)));
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
