MIME-Version: 1.0
References: <CAKFQuwawy1e6YR4S=j+y7pXqg_Dw1WBVrgvf=BP3d1_aSfe_+Q@mail.gmail.com>
 <CACJufxGsGqU0BnHw9+Vk3KhBSA3951fZs+JQnM387CiJD3Qn+g@mail.gmail.com>
 <CALdSSPh13qiKmhqwj=bR_3seZJVkP9E6BDbLfHXQ_DbaxQL4FA@mail.gmail.com>
 <CACJufxEq9aQX6ddkHeEX-RqjTZfDF02BLWGVS28ixRQQxoz0LA@mail.gmail.com>
 <CALdSSPjiR8cqPrJOjoGPvTnDufuWYj94-1whCGtGox=_QeZQEA@mail.gmail.com>
 <CACJufxEKq25CvQj4rYxcvHC_0MZQxzfwEKVWo9i-bKKxbVw3BA@mail.gmail.com>
 <CALdSSPgcuW1omVFUpnL6H63H+2qvtvEkD+4H3jUh1iEaNn_nEg@mail.gmail.com>
 <CACJufxFTvtMy_aYFQPtcBwVwJ-2=rJRVE0SBWv6efv86J5DyAQ@mail.gmail.com>
 <CALdSSPhxprTyzzr4tk-QU-GUroBKYNuJJ2GxWnExbB=bjQjJ7w@mail.gmail.com>
 <CACJufxHpTC-kqmW0LKNd_ZgnPJkhiAebPQkX2YE-CVxBP4LDfg@mail.gmail.com>
 <CALdSSPjpi0w6US75_st_-zOdUfr5q2vXmh08x0pL4FGJGYZvsQ@mail.gmail.com>
 <CACJufxE420KH=SLQSMEmi8QfEhXgVL8kGkuZJTviZ0o=TEVQgw@mail.gmail.com>
 <90dc6e9d-9348-485a-b27c-7b1637f06c2e@uni-muenster.de> <CACJufxFej2xUCERoWU+Eb+R+L_xzMNq4ZERNtL=SPF8O0T0ygw@mail.gmail.com>
 <cb26ae01-74d5-47b8-b257-c5e396339b2c@uni-muenster.de> <CACJufxHj0PejJC8BMq1cCiFC5uFdnazqrHh_VEm=s65-cNSnwQ@mail.gmail.com>
 <CALDaNm0Q3hA9w76gvB=goOY4ktKb-wjrbZbOcrqSwvxdm0yrcw@mail.gmail.com>
 <CACJufxEueK_MFebYqp73e+P+ykbD4+QbONptMg=iaaxUEGU7EQ@mail.gmail.com>
 <CALDaNm0FUtPjH9wz5dgwOBNtsAXEZj=0-TYGsyQXchQy-hXGrw@mail.gmail.com> <CACJufxFpm2Gzx4AD9qKtiQSiiiunX02wNTxu0JoFm7nEKF2KUw@mail.gmail.com>
In-Reply-To: <CACJufxFpm2Gzx4AD9qKtiQSiiiunX02wNTxu0JoFm7nEKF2KUw@mail.gmail.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Fri, 4 Apr 2025 14:32:24 -0700
Message-ID: <CAD21AoBjmEYjZT9A6P-vAuwboiiYtXaMc12pX2ySmh3RXi=v8w@mail.gmail.com>
Subject: Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row
To: jian he <jian.universality@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, Jim Jones <jim.jones@uni-muenster.de>, 
	Kirill Reshke <reshkekirill@gmail.com>, Fujii Masao <masao.fujii@oss.nttdata.com>, 
	"David G. Johnston" <david.g.johnston@gmail.com>, Yugo NAGATA <nagata@sraoss.co.jp>, 
	torikoshia <torikoshia@oss.nttdata.com>, 
	PostgreSQL Hackers <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/CAD21AoBjmEYjZT9A6P-vAuwboiiYtXaMc12pX2ySmh3RXi%3Dv8w%40mail.gmail.com>
Precedence: bulk

On Fri, Apr 4, 2025 at 4:55=E2=80=AFAM jian he <jian.universality@gmail.com=
> wrote:
>
> On Tue, Mar 25, 2025 at 2:31=E2=80=AFPM vignesh C <vignesh21@gmail.com> w=
rote:
> >
> > 2) Here in error we say column c1 violates not-null constraint and in
> > the context we show column c2, should the context also display c2
> > column:
> > postgres=3D# create table t3(c1 int not null, c2 int, check (c1 > 10));
> > CREATE TABLE
> > postgres=3D# COPY t3 FROM STDIN WITH (on_error set_to_null);
> > Enter data to be copied followed by a newline.
> > End with a backslash and a period on a line by itself, or an EOF signal=
.
> > >> a  b
> > >> \.
> > ERROR:  null value in column "c1" of relation "t3" violates not-null co=
nstraint
> > DETAIL:  Failing row contains (null, null).
> > CONTEXT:  COPY t3, line 1, column c2: "b"
> >
>
> It took me a while to figure out why.
> with the attached, now the error message becomes:
>
> ERROR:  null value in column "c1" of relation "t3" violates not-null cons=
traint
> DETAIL:  Failing row contains (null, null).
> CONTEXT:  COPY t3, line 1: "a,b"
>
> while at it,
> (on_error set_to_null, log_verbosity verbose)
> error message CONTEXT will only emit out relation name,
> this aligns with (on_error ignore, log_verbosity verbose).
>
> one of the message out example:
> +NOTICE:  column "b" was set to null due to data type incompatibility at =
line 2
> +CONTEXT:  COPY t_on_error_null
>
>
>
> > 3) typo becomen should be become:
> > null will becomen reserved to non-reserved
> fixed.
>
> > 4) There is a whitespace error while applying patch
> > Applying: COPY (on_error set_to_null)
> > .git/rebase-apply/patch:39: trailing whitespace.
> >       a <literal>NOTICE</literal> message indicating the number of rows
> > warning: 1 line adds whitespace errors.
> fixed.

I've reviewed the v15 patch and here are some comments:

How about renaming the new option value to 'set_null"? The 'to' in the
value name seems redundant to me.

---
+        COPY_ON_ERROR_NULL,                    /* set error field to null =
*/

I think it's better to rename COPY_ON_ERROR_SET_TO_NULL (or
COPY_ON_ERROR_SET_NULL if we change the option value name) for
consistency with the value name.

---
+                else if (cstate->opts.on_error =3D=3D COPY_ON_ERROR_NULL)
+                        ereport(NOTICE,
+                                        errmsg_plural("invalid values
in %" PRIu64 " row was replaced with null",
+
"invalid values in %" PRIu64 " rows were replaced with null",
+
cstate->num_errors,
+
cstate->num_errors));

How about adding "due to data type incompatibility" at the end of the messa=
ge?

---
+                                    ereport(NOTICE,
+                                                    errmsg("column
\"%s\" was set to null due to data type incompatibility at line %"
PRIu64 "",
+
cstate->cur_attname,
+
cstate->cur_lineno));

Similar to the IGNORE case, we can show the data in question in the message=
.

---
+                    else
+                            ereport(ERROR,
+
errcode(ERRCODE_NOT_NULL_VIOLATION),
+                                            errmsg("domain %s does
not allow null values", format_type_be(typioparams[m])),
+                                            errdatatype(typioparams[m]));

If domain data type is the sole case where not to accept NULL, can we
check it beforehand to avoid calling the second
InputFunctionCallSafe() for non-domain data types? Also, if we want to
end up with an error when setting NULL to a domain type with NOT NULL,
I think we don't need to try to handle a soft error by passing
econtext to InputFunctionCallSafe().

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com