Re: Add new error_action COPY ON_ERROR "log" - Mailing list pgsql-hackers

From torikoshia
Subject Re: Add new error_action COPY ON_ERROR "log"
Date
Msg-id 8e10b7c0021633338b85f9a7691c6b4b@oss.nttdata.com
Whole thread Raw
In response to Re: Add new error_action COPY ON_ERROR "log"  (Masahiko Sawada <sawada.mshk@gmail.com>)
Responses Re: Add new error_action COPY ON_ERROR "log"
List pgsql-hackers
On 2024-03-26 17:16, Masahiko Sawada wrote:
> On Tue, Mar 26, 2024 at 3:04 PM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
>> 
>> On Tue, Mar 26, 2024 at 9:56 AM Masahiko Sawada 
>> <sawada.mshk@gmail.com> wrote:
>> >
>> > > > errmsg("data type incompatibility at line %llu for column %s: \"%s\"",
>> >
>> > > > I guess it would be better to make the log message clearer to convey
>> > > > what we did for the malformed row. For example, how about something
>> > > > like "skipping row due to data type incompatibility at line %llu for
>> > > > column %s: \"s\""?
>> > >
>> > > The summary message which gets printed at the end says that "NOTICE:
>> > > 6 rows were skipped due to data type incompatibility". Isn't this
>> > > enough? If someone is using ON_ERROR 'ignore', it's quite natural that
>> > > such rows get skipped softly and the summary message can help them,
>> > > no?
>> >
>> > I think that in the main log message we should mention what happened
>> > (or is happening) or what we did (or are doing). If the message "data
>> > type incompatibility ..." was in the DETAIL message with the main
>> > message saying something like "skipping row at line %llu for column
>> > %s: ...", it would make sense to me. But the current message seems not
>> > to be clear to me and consistent with other NOTICE messages. Also, the
>> > last summary line would not be written if the user cancelled, and
>> > someone other than person who used ON_ERROR 'ignore' might check the
>> > server logs later.
>> 
>> Agree. I changed the NOTICE message to what you've suggested. Thanks.
>> 
> 
> Thank you for updating the patch! Looks good to me.
> 
> Please find the attached patch. I've made some changes for the
> documentation and the commit message. I'll push it, barring any
> objections.
> 
> Regards,
> 
> --
> Masahiko Sawada
> Amazon Web Services: https://aws.amazon.com

Thanks!

Attached patch fixes the doc, but I'm wondering perhaps it might be 
better to modify the codes to prohibit abbreviation of the value.

When seeing the query which abbreviates ON_ERROR value, I feel it's not 
obvious what happens compared to other options which tolerates 
abbreviation of the value such as FREEZE or HEADER.

   COPY t1 FROM stdin WITH (ON_ERROR);

What do you think?

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation



pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: [HACKERS] make async slave to wait for lsn to be replayed
Next
From: Bharath Rupireddy
Date:
Subject: Re: Add new error_action COPY ON_ERROR "log"