From: torikoshia <torikoshia@oss.nttdata.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Yugo NAGATA <nagata@sraoss.co.jp>
Cc: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Add reject_limit option to file_fdw
Date: Thu, 21 Nov 2024 15:43:22 +0900
Message-ID: <ed549b14a025b7f4b7451c035205d762@oss.nttdata.com> (raw)
In-Reply-To: <336dd0aa-f91b-4953-b0a0-b87215c571d2@oss.nttdata.com>
References: <bab68a9fc502b12693f0755b6f35f327@oss.nttdata.com>
<fd14c3a0-a036-4bce-b5f8-4af83259eae1@oss.nttdata.com>
<54747389a562688eab7077b144daff6a@oss.nttdata.com>
<2b3ad6d4-e05f-4188-8cfe-1fd9d949a9c9@oss.nttdata.com>
<192ede1e4c2b34b853e5ac46f8dee50b@oss.nttdata.com>
<d7a03664-bef5-49b0-998f-3ea5356b53c9@oss.nttdata.com>
<f02ec7b6c348b9fbeeb69657733da9ba@oss.nttdata.com>
<20241112145114.ba288defc684699bf8646942@sraoss.co.jp>
<2f03bf95556c4c41d6b1659e74b346c7@oss.nttdata.com>
<20241113231706.09e5b5ea9640289312835be8@sraoss.co.jp>
<a7d70e35229e2075b371cd1e81222244@oss.nttdata.com>
<336dd0aa-f91b-4953-b0a0-b87215c571d2@oss.nttdata.com>
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.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: torikoshia@oss.nttdata.com, masao.fujii@oss.nttdata.com, nagata@sraoss.co.jp
Subject: Re: Add reject_limit option to file_fdw
In-Reply-To: <ed549b14a025b7f4b7451c035205d762@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox