Re: Add new error_action COPY ON_ERROR "log"

From: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, jian(dot)universality(at)gmail(dot)com, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Subject: Re: Add new error_action COPY ON_ERROR "log"
Date: 2024-03-28 08:13:46
Message-ID: 8e10b7c0021633338b85f9a7691c6b4b@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2024-03-26 17:16, Masahiko Sawada wrote:
> On Tue, Mar 26, 2024 at 3:04 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>>
>> On Tue, Mar 26, 2024 at 9:56 AM Masahiko Sawada
>> <sawada(dot)mshk(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2024-03-28 08:27:51 Re: Add new error_action COPY ON_ERROR "log"
Previous Message Thomas Munro 2024-03-28 07:36:32 Re: [HACKERS] make async slave to wait for lsn to be replayed