On 2021/05/19 14:34, Bharath Rupireddy wrote:
On Wed, May 19, 2021 at 8:28 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
I agree with throwing an error for non-numeric junk though.
Allowing that on the grounds of backwards compatibility
seems like too much of a stretch.
+1.
+1.
+1
Thanks all for your inputs. PSA which uses parse_int for
batch_size/fech_size and parse_real for fdw_startup_cost and
fdw_tuple_cost.
Thanks for updating the patch! It basically looks good to me.
- val = strtod(defGetString(def), &endp);
- if (*endp || val < 0)
+ is_parsed = parse_real(defGetString(def), &val, 0,
NULL);
+ if (!is_parsed || val < 0)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("%s requires a non-negative
numeric value",
Isn't it better to check "!is_parsed" and "val < 0" separately like
reloptions.c does? That is, we should throw different error messages
for them?
ERRCODE_SYNTAX_ERROR seems strange for this type of error?
ERRCODE_INVALID_PARAMETER_VALUE is better and more proper?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
