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. 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. Regards, Bharath Rupireddy.
v4-0001-Tighten-up-batch_size-fetch_size-options-against-.patch
Description: Binary data