On 2024/10/03 13:23, Masahiko Sawada wrote:
On Tue, Oct 1, 2024 at 11:34 PM Fujii Masao <masao.fu...@oss.nttdata.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.

BTW, regarding the issue I mentioned earlier about file_fdw not reporting
the number of tuples processed and skipped in the pg_stat_progress_copy view,
I'll start a new thread to discuss this further.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION



Reply via email to