Message-ID: <501f4433-b820-4d26-b7f0-1ab9ccf393b3@oss.nttdata.com>
Date: Tue, 8 Oct 2024 18:39:54 +0900
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Add new COPY option REJECT_LIMIT
Content-Language: en-US
To: torikoshia <torikoshia@oss.nttdata.com>, jian.universality@gmail.com
Cc: zhjwpku@gmail.com, pgsql-hackers@lists.postgresql.org
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>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
In-Reply-To: <e12c970ed22e44d7600662d32afc374d@oss.nttdata.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/501f4433-b820-4d26-b7f0-1ab9ccf393b3%40oss.nttdata.com>
Precedence: bulk


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.


> 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?

Regards,

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