MIME-Version: 1.0
References: <CACJufxFFU92-H7G8tmpxt9oTSfL062OA7n5rPx-YbOAtDUUzGw@mail.gmail.com>
 <04bf425ad1b15a4daefe96c478a5253b@oss.nttdata.com> <20240205172256.0942c4e8b757fc6f7c4dbfa8@sraoss.co.jp>
 <20240206.093909.1866377550429411676.horikyota.ntt@gmail.com> <20240206164641.43d0793c5d82da27b5982ef5@sraoss.co.jp>
In-Reply-To: <20240206164641.43d0793c5d82da27b5982ef5@sraoss.co.jp>
From: jian he <jian.universality@gmail.com>
Date: Tue, 6 Feb 2024 16:38:31 +0800
Message-ID: <CACJufxHUC8=aRCXHXa4n=6peiN_r8uvVahOTtG_+xq2=+NJMmg@mail.gmail.com>
Subject: Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row
To: Yugo NAGATA <nagata@sraoss.co.jp>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, torikoshia@oss.nttdata.com, 
	david.g.johnston@gmail.com, pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CACJufxHUC8%3DaRCXHXa4n%3D6peiN_r8uvVahOTtG_%2Bxq2%3D%2BNJMmg%40mail.gmail.com>
Precedence: bulk

On Tue, Feb 6, 2024 at 3:46=E2=80=AFPM Yugo NAGATA <nagata@sraoss.co.jp> wr=
ote:
>
> On Tue, 06 Feb 2024 09:39:09 +0900 (JST)
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>
> > At Mon, 5 Feb 2024 17:22:56 +0900, Yugo NAGATA <nagata@sraoss.co.jp> wr=
ote in
> > > On Mon, 05 Feb 2024 11:28:59 +0900
> > > torikoshia <torikoshia@oss.nttdata.com> wrote:
> > >
> > > > > Based on this, I've made a patch.
> > > > > based on COPY Synopsis: ON_ERROR 'error_action'
> > > > > on_error 'null', the  keyword NULL should be single quoted.
> > > >
> > > > As you mentioned, single quotation seems a little odd..
> > > >
> > > > I'm not sure what is the best name and syntax for this feature, but
> > > > since current error_action are verbs('stop' and 'ignore'), I feel '=
null'
> > > > might not be appropriate.
> > >
> > > I am not in favour of using 'null' either, so I suggested to use
> > > "set_to_null" or more generic syntax like  "set_to (col, val)" in my
> > > previous post[1], although I'm not convinced what is the best either.
> > >
> > > [1] https://www.postgresql.org/message-id/20240129172858.ccb6c77c3be9=
5a295e2b2b44%40sraoss.co.jp
> >
> > Tom sugggested using a separate option, and I agree with the
> > suggestion. Taking this into consideration, I imagined something like
> > the following, for example.  Although I'm not sure we are actually
> > going to do whole-tuple replacement, the action name in this example
> > has the suffix '-column'.
> >
> > COPY (on_error 'replace-colomn', replacement 'null') ..
>
> Thank you for your information. I've found a post[1] you mentioned,
> where adding a separate option for error log destination was suggested.
>
> Considering consistency with other options, adding a separate option
> would be better if we want to specify a value to replace the invalid
> value, without introducing a complex syntax that allows options with
> more than one parameters. Maybe, if we allow to use values for the
> replacement other than NULL, we have to also add a option to specify
> a column (or a type)  for each replacement value. Or,  we may add a
> option to specify a list of replacement values as many as the number of
> columns, each of whose default is NULL.
>
> Anyway, I prefer 'replace" (or 'set_to') to just 'null' as the option
> value.
>

Let's say tabe t column (a,b,c)
if we support set_to_null(a,b), what should we do if column c has an error.
should we ignore this row or error out immediately?
also I am not sure it's doable to just extract columnList from the
function defGetCopyOnErrorChoice.

to make `COPY x from stdin (on_error set_to_null(a,b);` work,
we may need to refactor to gram.y, in a similar way we do force null

i am ok with
COPY x from stdin (on_error set_to_null);