public inbox for [email protected]  

help / color / mirror / Atom feed

From: Fujii Masao <[email protected]>
To: Masahiko Sawada <[email protected]>
Cc: torikoshia <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Pgsql Hackers <[email protected]>
Subject: Re: Add on_error and log_verbosity options to file_fdw
Date: Thu, 3 Oct 2024 18:03:48 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAD21AoC=0ZrEe0QDNxomC=xFxbCsf-wjSdNaR8DoCWRMfvvE2A@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CAD21AoBij_2cy5T7ihk9ArRQzDcXVHoK13vzy34zAUAMLgrKkQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAD21AoCt4UTGs-V_sGewCdjvmjhgnnOUG3T83tRF=VtbdW-dPg@mail.gmail.com>
	<[email protected]>
	<CAD21AoC=0ZrEe0QDNxomC=xFxbCsf-wjSdNaR8DoCWRMfvvE2A@mail.gmail.com>



On 2024/10/03 13:23, Masahiko Sawada wrote:
> On Tue, Oct 1, 2024 at 11:34 PM Fujii Masao <[email protected]> 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

view thread (7+ messages)  latest in thread

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add on_error and log_verbosity options to file_fdw
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox