Re: Add reject_limit option to file_fdw

On 2024-11-21 00:43, Fujii Masao wrote:
> On 2024/11/19 21:40, torikoshia wrote:
>>> These messages may be unexpected for some users because the 
>>> documentation of
>>> fild_fdw does not explicitly describe that file_fdw uses COPY 
>>> internally.
>>> (I can find several wordings like "as COPY", though.)
>>> However, since the current file_fdw already has such messages, I came 
>>> to think
>>> making the messages on "reject_limit" consistent with COPY is 
>>> reasonable.
>>> I mean, the previous version of the message seems fine.
>> 
>> Agreed. Thanks for your investigation.
> 
> I've pushed the 0001 patch. Thanks!

Thanks a lot!

>>> As another comment, do we need to add validator test for on_error and
>>> reject_limit as similar to other options?
>> 
>> That might be better.
>> Added some queries which had wrong options to cause errors.
>> 
>> I was unsure whether to add it to "validator test" section or 
>> "on_error, log_verbosity and reject_limit tests" section, but I chose 
>> "validator test" as I believe it makes things clearer.
>> 
>> Added 0002 patch since some of the tests are not related to 
>> reject_limit but just on_error.
> 
> How about adding validator tests for log_verbosity and reject_limit=0,
> similar to the tests in the copy2.sql regression test? I've added those
> two tests, and the latest version of the patch is attached.

Thanks for the update! Agreed.
I think the validator tests related to the on_error option you added are 
sufficient.

As for the other file_fdw options, I'm a bit concerned whether it would 
be better to add validator tests for cases like when 'encoding' is 
'unsupported', but that should be discussed in another thread.

> Regards,

-- 
Regards,

--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.