On 2024/10/17 22:45, torikoshia wrote:
> Hi,
>
> 4ac2a9bec introduced reject_limit option to the COPY command, and I was wondering if it might be beneficial to add
thesame option to file_fdw.
>
> Although there may be fewer practical use cases compared to COPY, it could still be useful in situations where the
filebeing read via file_fdw is subject to modifications and there is a need to tolerate a limited number of errors.
Agreed.
>
> What do you think?
>
> I've attached a patch.
Thanks for the patch! Could you add it to the next CommitFest?
+ALTER FOREIGN TABLE agg_bad OPTIONS (reject_limit '1');
+SELECT * FROM agg_bad;
+ a | b
+-----+--------
+ 100 | 99.097
+ 42 | 324.78
+(2 rows)
Wouldn't it be better to include a test where a SELECT query fails, even with
on_error set to "ignore," because the number of errors exceeds reject_limit?
+ if (cstate->opts.reject_limit > 0 && \
The trailing backslash isn't needed here.
+ <varlistentry>
+ <term><literal>reject_limit</literal></term>
This entry should be placed right after the on_error option,
following the same order as in the COPY command documentation.
> Based on the synopsis of the CREATE/ALTER FOREIGN TABLE commands, the value for the foreign table's option must be
single-quoted.I’m not entirely sure if this is the correct approach, but in order to accommodate this, the patch
modifiesthe value of reject_limit option to accept not only numeric values but also strings.
>
I don't have a better approach for this, so I'm okay with your solution.
Just one note: it would be helpful to explain and comment
why defGetCopyRejectLimitOption() accepts and parses both int64 and
string values.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
