MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Mon, 05 Feb 2024 11:28:59 +0900
From: torikoshia <torikoshia@oss.nttdata.com>
To: jian he <jian.universality@gmail.com>
Cc: Yugo NAGATA <nagata@sraoss.co.jp>, "David G. Johnston"
 <david.g.johnston@gmail.com>, PostgreSQL Hackers
 <pgsql-hackers@lists.postgresql.org>
Subject: Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row
In-Reply-To: <CACJufxFFU92-H7G8tmpxt9oTSfL062OA7n5rPx-YbOAtDUUzGw@mail.gmail.com>
References: <CAKFQuwawy1e6YR4S=j+y7pXqg_Dw1WBVrgvf=BP3d1_aSfe_+Q@mail.gmail.com>
 <20240129172858.ccb6c77c3be95a295e2b2b44@sraoss.co.jp>
 <CACJufxFFU92-H7G8tmpxt9oTSfL062OA7n5rPx-YbOAtDUUzGw@mail.gmail.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <04bf425ad1b15a4daefe96c478a5253b@oss.nttdata.com>
Archived-At: <https://www.postgresql.org/message-id/04bf425ad1b15a4daefe96c478a5253b%40oss.nttdata.com>
Precedence: bulk

Hi,

On 2024-02-03 15:22, jian he wrote:
> The idea of on_error is to tolerate errors, I think.
> if a column has a not null constraint, let it cannot be used with
> (on_error 'null')

> +       /*
> +        * we can specify on_error 'null', but it can only apply to 
> columns
> +        * don't have not null constraint.
> +       */
> +       if (att->attnotnull && cstate->opts.on_error == 
> COPY_ON_ERROR_NULL)
> +           ereport(ERROR,
> +                   (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
> +                    errmsg("copy on_error 'null' cannot be used with 
> not null constraint column")));

This means we cannot use ON_ERROR 'null' even when there is one column 
which have NOT NULL constraint, i.e. primary key, right?
IMHO this is strong constraint and will decrease the opportunity to use 
this feature.

It might be better to allow error_action 'null' for tables which have 
NOT NULL constraint columns, and when facing soft errors for those rows, 
skip that row or stop COPY.

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

> demo:
> COPY check_ign_err FROM STDIN WITH (on_error 'null');
> 1 {1} a
> 2 {2} 1
> 3 {3} 2
> 4 {4} b
> a {5} c
> \.
> 
> \pset null NULL
> 
> SELECT * FROM check_ign_err;
>   n   |  m  |  k
> ------+-----+------
>     1 | {1} | NULL
>     2 | {2} |    1
>     3 | {3} |    2
>     4 | {4} | NULL
>  NULL | {5} | NULL

Since we notice the number of ignored rows when ON_ERROR is 'ignore', 
users may want to know the number of rows which was changed to NULL when 
using ON_ERROR 'null'.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation