Re: postgres_fdw - should we tighten up batch_size, fetch_size options against non-numeric values?

On Mon, May 17, 2021 at 7:50 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> If we do that, then the options that are only accepting unquoted
> integers (i.e. 123, 456 etc.) and throwing errors for the quoted
> integers ('123', '456' etc.) will then start to accept the quoted
> integers. Other hackers might not agree to this change.

I guess the options which weren't accepting quoted strings, will catch
these errors at the time of parsing itself. Even if that's not true, I
would see that as an improvement. Anyway, I won't stretch this
further.

>
> Another way is to have new API like
> defGetInt32_2/defGetInt64_2/defGetNumeric_2 (or some other better
> names) which basically accept both quoted and unquoted strings, see
> [1] for a rough sketch of the function. These API can be useful if an
> option needs to be parsed in both quoted and unquoted form. Or we can
> also have these functions as [2] for only parsing quoted options. I
> prefer [2] so that these API can be used without any code duplication.
> Thoughts?

I am not sure whether we want to maintain two copies. In that case
your first patch is fine.

> Note
> that I have not added any test case as this change is a trivial thing.
> If required, I can add one.

That will help to make sure that we preserve the behaviour even
through code changes.

--
Best Wishes,
Ashutosh Bapat