MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Tue, 08 Oct 2024 21:58:51 +0900
From: torikoshia <torikoshia@oss.nttdata.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: jian.universality@gmail.com, zhjwpku@gmail.com,
 pgsql-hackers@lists.postgresql.org
Subject: Re: Add new COPY option REJECT_LIMIT
In-Reply-To: <501f4433-b820-4d26-b7f0-1ab9ccf393b3@oss.nttdata.com>
References: <63f99327aa6b404cc951217fa3e61fe4@oss.nttdata.com>
 <fab7018f-6e00-4be4-bd1e-4a013096f6c9@oss.nttdata.com>
 <de0551d0f9d9e9072e324e51ed5c426d@oss.nttdata.com>
 <6e3dcdc6-8e18-4716-a23b-3bee316aeaa7@oss.nttdata.com>
 <979c6d306343f3d665d14eab2cd1b7bc@oss.nttdata.com>
 <920a29a36032befbeb355cbdfbbe0f63@oss.nttdata.com>
 <a873643c-e7d1-456c-b16b-6162ab698036@oss.nttdata.com>
 <54bea7dd-94b1-41f1-a4e9-c5c53cc82ce7@oss.nttdata.com>
 <f9feda0ecd4e2e09620a6f9ca8f4aac1@oss.nttdata.com>
 <644a094c-2c0c-4b5f-b151-3677c7de4ee7@oss.nttdata.com>
 <5f807fcf3a36df7ba41464ab40b5c37d@oss.nttdata.com>
 <056a5728cf733685cff7d9b5a8fbb0ad@oss.nttdata.com>
 <a0343e25-644e-4435-8399-f2e74c04ca23@oss.nttdata.com>
 <dd542f3ce7e15cde7e53824041f2a2f4@oss.nttdata.com>
 <CACJufxEh5e_5BOmP7RAHcuV=e32FpzBMxJ8-W-OFXm-zkdY4yw@mail.gmail.com>
 <d9636900e5a1b7117f5f42daf9be8cab@oss.nttdata.com>
 <8830518a-28ac-43a2-8a11-1676d9a3cdf8@oss.nttdata.com>
 <e12c970ed22e44d7600662d32afc374d@oss.nttdata.com>
 <501f4433-b820-4d26-b7f0-1ab9ccf393b3@oss.nttdata.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <98559893e4475fba60c2130a2178a84b@oss.nttdata.com>
Archived-At: 
 <https://www.postgresql.org/message-id/98559893e4475fba60c2130a2178a84b%40oss.nttdata.com>
Precedence: bulk

On 2024-10-08 18:39, Fujii Masao wrote:
> On 2024/10/07 21:51, torikoshia wrote:
>>> While reviewing, I also noticed that the check for
>>> "opts_out->binary && opts_out->on_error != COPY_ON_ERROR_STOP"
>>> is similarly placed before setting the defaults, which might not
>>> be correct. This check should probably be moved as well.
>>> Additionally, the comment mentioning "must do these two" should be
>>> updated to "must do these three." These changes should be handled
>>> in a separate patch.
>> 
>> Agreed and attached 0002 patch.
> 
> Thanks for updating the 0001 patch and creating the 0002 patch! I've
> pushed both.

Thanks a lot!

> 
> 
>> Also considering when REJECT_LIMIT is specified to 1, attached patch 
>> uses errmsg_plural() instead of errmsg.
> 
> I don't think errmsg_plural() is needed here since, when 1 is 
> specified,
> "rows" should follow "more than REJECT_LIMIT (1)". No?

You are right.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation