MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Date: Thu, 03 Oct 2024 23:17:40 +0900
From: torikoshia <torikoshia@oss.nttdata.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>, sawada.mshk@gmail.com
Cc: Michael Paquier <michael@paquier.xyz>, Pgsql Hackers
 <pgsql-hackers@postgresql.org>
Subject: Re: Add on_error and log_verbosity options to file_fdw
In-Reply-To: <bcfda343-69bb-4973-b95e-99f4ed9f49e2@oss.nttdata.com>
References: <ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com>
 <841721f3ca9a69574a72eb141c83933c@oss.nttdata.com>
 <CAD21AoBij_2cy5T7ihk9ArRQzDcXVHoK13vzy34zAUAMLgrKkQ@mail.gmail.com>
 <Zp7x8Y-SpbkZtpE-@paquier.xyz>
 <c22f047faf8aea56acecb26449b3f454@oss.nttdata.com>
 <79a336ccebc70401625743686c41b81a@oss.nttdata.com>
 <b0d44cd9-a7f7-469c-9ecf-b0115b86ca70@oss.nttdata.com>
 <5739a8ee4320a31ca04752fe3972aa73@oss.nttdata.com>
 <81844000-703c-408a-9296-14f5e176098d@oss.nttdata.com>
 <156b2c1bfe27441aa5e59a52d0af5142@oss.nttdata.com>
 <1b24fcc9-0f18-40bf-b532-ab4cbe4b9e32@oss.nttdata.com>
 <6383a9a5b90a36dbe60c6f30286d2254@oss.nttdata.com>
 <bbddc7a0-b71a-4d54-9560-c90a26a06c48@oss.nttdata.com>
 <CAD21AoCt4UTGs-V_sGewCdjvmjhgnnOUG3T83tRF=VtbdW-dPg@mail.gmail.com>
 <9fb4e232-88dc-4554-8a4f-ba307a52f9da@oss.nttdata.com>
 <CAD21AoC=0ZrEe0QDNxomC=xFxbCsf-wjSdNaR8DoCWRMfvvE2A@mail.gmail.com>
 <bcfda343-69bb-4973-b95e-99f4ed9f49e2@oss.nttdata.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <70dc3a5091a81ffc8c3be367b4b55efc@oss.nttdata.com>
Archived-At: <https://www.postgresql.org/message-id/70dc3a5091a81ffc8c3be367b4b55efc%40oss.nttdata.com>
Precedence: bulk

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.fujii@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.

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


-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation