public inbox for [email protected]  
help / color / mirror / Atom feed
From: jian he <[email protected]>
To: Fujii Masao <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: Jim Jones <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Yugo NAGATA <[email protected]>
Cc: torikoshia <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row
Date: Sun, 27 Oct 2024 19:32:45 +0800
Message-ID: <CACJufxHeZ-NSqwtY7cG9WO-wfnGM2rbGB9k+9pfRX8zUybMSyA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAKFQuwawy1e6YR4S=j+y7pXqg_Dw1WBVrgvf=BP3d1_aSfe_+Q@mail.gmail.com>
	<[email protected]>
	<CACJufxFFU92-H7G8tmpxt9oTSfL062OA7n5rPx-YbOAtDUUzGw@mail.gmail.com>
	<[email protected]>
	<CACJufxGnc+=No=Ua6NFT2ADt0vRL=m1QsuCOM=9aKPKWh9_L6Q@mail.gmail.com>
	<[email protected]>
	<CACJufxFT9j8o5kEC8dPCQqLomWjeJm9V9m8eZjj2Gvc_F5ha=g@mail.gmail.com>
	<[email protected]>
	<CAKFQuwaYNw8U-9JkFdyOX4i4Y3J1sp6+dk-sh8YmZGCq8gMeVQ@mail.gmail.com>
	<[email protected]>
	<CACJufxFFdtPKk4B5rSVNEk6yCH2Amvi_8w3Gaz5wg9M_t9c5Rw@mail.gmail.com>
	<[email protected]>
	<CACJufxEgiysa2SMJPGp0aN476Ojm636MfJK88DZC7TVYsXYBBQ@mail.gmail.com>
	<CALdSSPhgjCbyb=ZRgr4LaCFJV2-F9_CxMeX6poHuGCt_f9GYAw@mail.gmail.com>
	<[email protected]>

On Mon, Oct 21, 2024 at 8:39 PM Fujii Masao <[email protected]> wrote:
> On 2024/10/21 18:30, Kirill Reshke wrote:
> > v4 no longer applies. It now conflicts with
> > e7834a1a251d4a28245377f383ff20a657ba8262.
> > Also, there were review comments.
> >
> > So, I decided to rebase.
>
> Thanks for the patch! Here are my review comments:
>
> I noticed that on_error=set_to_null does not trigger NOTICE messages for rows
> and columns with errors. It's "unexpected" thing for columns to be silently
> replaced with NULL due to on_error=set_to_null. So, similar to on_error=ignore,
> there should be NOTICE messages indicating which input records had columns
> set to NULL because of data type incompatibility. Without these messages,
> users might not realize that some columns were set to NULL.
>

on_error=set_to_null,
we have two options for CopyFromStateData->num_errors.
A. Counting the number of rows that on_error set_to_null happened.
B. Counting number of times that on_error set_to_null happened

let's say optionA:
        ereport(NOTICE,
                errmsg_plural("%llu row was converted to NULL due to
data type incompatibility",
                              "%llu rows were converted to NULL due to
data type incompatibility",
                              (unsigned long long) cstate->num_errors,
                              (unsigned long long) cstate->num_errors));

I doubt the above message is accurate.
"%llu row was converted to NULL"
can mean "%llu row, for each row, all columns was converted to NULL"
but here we are
"%llu row, for each row, some column (can be all columns) was converted to NULL"


optionB: the message can be:
errmsg_plural("converted to NULL due to data type incompatibility
happened %llu time")
but I aslo feel the wording is not perfect also.

So overall I am not sure how to construct the NOTICE messages.






view thread (29+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row
  In-Reply-To: <CACJufxHeZ-NSqwtY7cG9WO-wfnGM2rbGB9k+9pfRX8zUybMSyA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox