On 2021/06/30 23:31, Bharath Rupireddy wrote:
On Wed, Jun 30, 2021 at 5:53 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
On 2021/05/20 1:01, Bharath Rupireddy wrote:
Thanks for the comments. I added separate messages, changed the error
code from ERRCODE_SYNTAX_ERROR to ERRCODE_INVALID_PARAMETER_VALUE and
also quoted the option name in the error message. PSA v3 patch.
Thanks for updating the patch!
+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid numeric value for option
\"%s\"",
+ def->defname)));
In reloptions.c, when parse_real() fails to parse the input, the error message
"invalid value for floating point option..." is output.
For the sake of consistency, we should use the same error message here?
Actually, there's an existing error message errmsg("%s requires a
non-negative numeric value" that used "numeric value". If we were to
change errmsg("invalid numeric value for option \"%s\"", to
errmsg("invalid value for floating point option \"%s\"",, then we
might have to change the existing message. And also, the docs use
"numeric value" for fdw_startup_cost and fdw_tuple_cost.
The recent commit 61d599ede7 documented that the type of those options is
floating point. But the docs still use "is a numeric value" in the descriptions
of them. Probably it should be replaced with "is a floating point value" there.
If we do this, isn't it better to use "floating point" even in the error
message?
IMO, let's go
with errmsg("invalid value for numeric option \"%s\": %s",.
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("%s requires a non-negative
integer value",
+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid integer value for option
\"%s\"",
IMO the error message should be "invalid value for integer option..." here
because of the same reason I told above. Thought?
Changed.
PSA v4.
Thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
