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. Thoughts? > 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. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
