On Tue, May 18, 2021 at 7:46 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Tue, May 18, 2021 at 7:19 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > Fujii Masao <masao.fu...@oss.nttdata.com> writes: > > > On 2021/05/17 18:58, Bharath Rupireddy wrote: > > >> It looks like the values such as '123.456', '789.123' '100$%$#$#', > > >> '9,223,372,' are accepted and treated as valid integers for > > >> postgres_fdw options batch_size and fetch_size. Whereas this is not > > >> the case with fdw_startup_cost and fdw_tuple_cost options for which an > > >> error is thrown. Attaching a patch to fix that. > > > > > This looks an improvement. But one issue is that the restore of > > > dump file taken by pg_dump from v13 may fail for v14 with this patch > > > if it contains invalid setting of fetch_size, e.g., "fetch_size > > > '123.456'". > > > OTOH, since batch_size was added in v14, it has no such issue. > > > > Maybe better to just silently round to integer? I think that's > > what we generally do with integer GUCs these days, eg > > > > regression=# set work_mem = 102.9; > > SET > > regression=# show work_mem; > > work_mem > > ---------- > > 103kB > > (1 row) > > I think we can use parse_int to parse the fetch_size and batch_size as > integers, which also rounds off decimals to integers and returns false > for non-numeric junk. But, it accepts both quoted and unquoted > integers, something like batch_size 100 and batch_size '100', which I > feel is okay because the reloptions also accept both. > > While on this, we can also use parse_real for fdw_startup_cost and > fdw_tuple_cost options but with that they will accept both quoted and > unquoted real values.
I'm sorry about saying that the unquoted integers are accepted with batch_size, fetch_size, but actually the parser is throwing the syntax error. So, we can safely use parse_int for batch_size and fetch_size, parse_real for fdw_tuple_cost and fdw_startup_cost without changing any behaviour. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com