Re: Add on_error and log_verbosity options to file_fdw

On 2024-10-03 18:03, Fujii Masao wrote:
> On 2024/10/03 13:23, Masahiko Sawada wrote:
>> On Tue, Oct 1, 2024 at 11:34 PM Fujii Masao
>> <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>>
>>>
>>> On 2024/10/02 9:27, Masahiko Sawada wrote:
>>>> Sorry for being late in joining the review of this patch. Both 0001
>>>> and 0003 look good to me. I have two comments on the 0002 patch:
>>>
>>> Thanks for the review!
>>>
>>>> I think that while scanning a file_fdw foreign table with
>>>> log_verbosity='silent' the query is not interruptible.
>>>
>>> You're right. I added CHECK_FOR_INTERRUPTS() in the retry loop.
>>>
>>>> Also, we don't switch to the per-tuple memory context when retrying
>>>> due to a soft error. I'm not sure it's okay as in CopyFrom(), a
>>>> similar function for COPY command, we switch to the per-tuple memory
>>>> context every time before parsing an input line. Would it be
>>>> problematic if we switch to another memory context while parsing an
>>>> input line? In CopyFrom() we also call ResetPerTupleExprContext()
>>>> and
>>>> ExecClearTuple() for every input, so we might want to consider
>>>> calling
>>>> them for every input.
>>>
>>> Yes, I've updated the patch based on your comment.
>>> Could you please review the latest version?
>>
>> Thank you for updating the patch! All patches look good to me.
>
> Thanks for the review! Pushed.

Fujii-san, Sawada-san,
Thanks for your review and modifications!

--
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation